Skip to content

Commit

Permalink
Fixed Issue of Review Menu Shown Behind (#32631)
Browse files Browse the repository at this point in the history
Fixed #31144 

---------

Co-authored-by: wxiaoguang <[email protected]>
  • Loading branch information
kerwin612 and wxiaoguang authored Nov 26, 2024
1 parent 703be6b commit 25cacaf
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 26 deletions.
30 changes: 14 additions & 16 deletions templates/repo/diff/box.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -164,24 +164,22 @@
<input type="checkbox" name="{{$file.GetDiffFileName}}" autocomplete="off"{{if $file.IsViewed}} checked{{end}}> {{ctx.Locale.Tr "repo.pulls.has_viewed_file"}}
</label>
{{end}}
<div class="ui dropdown basic">
{{svg "octicon-kebab-horizontal" 18 "icon tw-mx-2"}}
<div class="ui menu">
{{if not (or $file.IsIncomplete $file.IsBin $file.IsSubmodule)}}
<button class="unescape-button item">{{ctx.Locale.Tr "repo.unescape_control_characters"}}</button>
<button class="escape-button tw-hidden item">{{ctx.Locale.Tr "repo.escape_control_characters"}}</button>
{{end}}
{{if and (not $file.IsSubmodule) (not $.PageIsWiki)}}
{{if $file.IsDeleted}}
<a class="item" rel="nofollow" href="{{$.BeforeSourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
{{else}}
<a class="item" rel="nofollow" href="{{$.SourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
{{if and $.Repository.CanEnableEditor $.CanEditFile (not $file.IsLFSFile) (not $file.IsBin)}}
<a class="item" rel="nofollow" href="{{$.HeadRepoLink}}/_edit/{{PathEscapeSegments $.HeadBranchName}}/{{PathEscapeSegments $file.Name}}?return_uri={{print $.BackToLink "#diff-" $file.NameHash | QueryEscape}}">{{ctx.Locale.Tr "repo.editor.edit_this_file"}}</a>
{{end}}
<button class="btn diff-header-popup-btn tw-p-1">{{svg "octicon-kebab-horizontal" 18}}</button>
<div class="tippy-target">
{{if not (or $file.IsIncomplete $file.IsBin $file.IsSubmodule)}}
<button class="unescape-button item" data-file-content-elem-id="diff-{{$file.NameHash}}">{{ctx.Locale.Tr "repo.unescape_control_characters"}}</button>
<button class="escape-button tw-hidden item" data-file-content-elem-id="diff-{{$file.NameHash}}">{{ctx.Locale.Tr "repo.escape_control_characters"}}</button>
{{end}}
{{if and (not $file.IsSubmodule) (not $.PageIsWiki)}}
{{if $file.IsDeleted}}
<a class="item" rel="nofollow" href="{{$.BeforeSourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
{{else}}
<a class="item" rel="nofollow" href="{{$.SourcePath}}/{{PathEscapeSegments .Name}}">{{ctx.Locale.Tr "repo.diff.view_file"}}</a>
{{if and $.Repository.CanEnableEditor $.CanEditFile (not $file.IsLFSFile) (not $file.IsBin)}}
<a class="item" rel="nofollow" href="{{$.HeadRepoLink}}/_edit/{{PathEscapeSegments $.HeadBranchName}}/{{PathEscapeSegments $file.Name}}?return_uri={{print $.BackToLink "#diff-" $file.NameHash | QueryEscape}}">{{ctx.Locale.Tr "repo.editor.edit_this_file"}}</a>
{{end}}
{{end}}
</div>
{{end}}
</div>
</div>
</h4>
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/pull_compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func TestPullCompare_EnableAllowEditsFromMaintainer(t *testing.T) {
user2Session := loginUser(t, "user2")
resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
nodes := htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a")
nodes := htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .tippy-target a")
if assert.Equal(t, 1, nodes.Length()) {
// there is only "View File" button, no "Edit File" button
assert.Equal(t, "View File", nodes.First().Text())
Expand All @@ -121,7 +121,7 @@ func TestPullCompare_EnableAllowEditsFromMaintainer(t *testing.T) {
// user2 (admin of repo3) goes to the PR files page again
resp = user2Session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("%s/files", prURL)), http.StatusOK)
htmlDoc = NewHTMLParser(t, resp.Body)
nodes = htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .dropdown .menu a")
nodes = htmlDoc.doc.Find(".diff-file-box[data-new-filename=\"README.md\"] .diff-file-header-actions .tippy-target a")
if assert.Equal(t, 2, nodes.Length()) {
// there are "View File" button and "Edit File" button
assert.Equal(t, "View File", nodes.First().Text())
Expand Down
2 changes: 2 additions & 0 deletions web_src/css/modules/tippy.css
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,10 @@
align-items: center;
padding: 9px 18px;
color: inherit;
background: inherit;
text-decoration: none;
gap: 10px;
width: 100%;
}

.tippy-box[data-theme="menu"] .item:hover {
Expand Down
12 changes: 12 additions & 0 deletions web_src/js/features/repo-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from '../utils/dom.ts';
import {POST, GET} from '../modules/fetch.ts';
import {fomanticQuery} from '../modules/fomantic/base.ts';
import {createTippy} from '../modules/tippy.ts';

const {pageData, i18n} = window.config;

Expand Down Expand Up @@ -140,12 +141,22 @@ export function initRepoDiffConversationNav() {
});
}

function initDiffHeaderPopup() {
for (const btn of document.querySelectorAll('.diff-header-popup-btn:not([data-header-popup-initialized])')) {
btn.setAttribute('data-header-popup-initialized', '');
const popup = btn.nextElementSibling;
if (!popup?.matches('.tippy-target')) throw new Error('Popup element not found');
createTippy(btn, {content: popup, theme: 'menu', placement: 'bottom', trigger: 'click', interactive: true, hideOnClick: true});
}
}

// Will be called when the show more (files) button has been pressed
function onShowMoreFiles() {
initRepoIssueContentHistory();
initViewedCheckboxListenerFor();
countAndUpdateViewedFiles();
initImageDiff();
initDiffHeaderPopup();
}

export async function loadMoreFiles(url) {
Expand Down Expand Up @@ -221,6 +232,7 @@ export function initRepoDiffView() {
initDiffFileList();
initDiffCommitSelect();
initRepoDiffShowMore();
initDiffHeaderPopup();
initRepoDiffFileViewToggle();
initViewedCheckboxListenerFor();
initExpandAndCollapseFilesButton();
Expand Down
12 changes: 6 additions & 6 deletions web_src/js/features/repo-unicode-escape.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import {hideElem, queryElemSiblings, showElem, toggleElem} from '../utils/dom.ts';
import {addDelegatedEventListener, hideElem, queryElemSiblings, showElem, toggleElem} from '../utils/dom.ts';

export function initUnicodeEscapeButton() {
document.addEventListener('click', (e) => {
const btn = e.target.closest('.escape-button, .unescape-button, .toggle-escape-button');
if (!btn) return;

addDelegatedEventListener(document, 'click', '.escape-button, .unescape-button, .toggle-escape-button', (btn, e) => {
e.preventDefault();

const fileContent = btn.closest('.file-content, .non-diff-file-content');
const fileContentElemId = btn.getAttribute('data-file-content-elem-id');
const fileContent = fileContentElemId ?
document.querySelector(`#${fileContentElemId}`) :
btn.closest('.file-content, .non-diff-file-content');
const fileView = fileContent?.querySelectorAll('.file-code, .file-view');
if (btn.matches('.escape-button')) {
for (const el of fileView) el.classList.add('unicode-escaped');
Expand Down
4 changes: 2 additions & 2 deletions web_src/js/utils/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import {debounce} from 'throttle-debounce';
import type {Promisable} from 'type-fest';
import type $ from 'jquery';

type ElementArg = Element | string | NodeListOf<Element> | Array<Element> | ReturnType<typeof $>;
type ArrayLikeIterable<T> = ArrayLike<T> & Iterable<T>; // for NodeListOf and Array
type ElementArg = Element | string | ArrayLikeIterable<Element> | ReturnType<typeof $>;
type ElementsCallback<T extends Element> = (el: T) => Promisable<any>;
type ElementsCallbackWithArgs = (el: Element, ...args: any[]) => Promisable<any>;
type ArrayLikeIterable<T> = ArrayLike<T> & Iterable<T>; // for NodeListOf and Array

function elementsCall(el: ElementArg, func: ElementsCallbackWithArgs, ...args: any[]) {
if (typeof el === 'string' || el instanceof String) {
Expand Down

0 comments on commit 25cacaf

Please sign in to comment.