Skip to content

Fix dropdown delegating and some UI problems #34014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 26, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/templates/util_avatar.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func AvatarHTML(src string, size int, class, name string) template.HTML {
name = "avatar"
}

return template.HTML(`<img loading="lazy" class="` + class + `" src="` + src + `" title="` + html.EscapeString(name) + `" width="` + sizeStr + `" height="` + sizeStr + `"/>`)
return template.HTML(`<img loading="lazy" alt="" class="` + class + `" src="` + src + `" title="` + html.EscapeString(name) + `" width="` + sizeStr + `" height="` + sizeStr + `"/>`)
}

// Avatar renders user avatars. args: user, size (int), class (string)
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/icon.tmpl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{{$avatarLink := (.RelAvatarLink ctx)}}
{{if $avatarLink}}
<img class="ui avatar tw-align-middle" src="{{$avatarLink}}" width="24" height="24" alt="{{.FullName}}">
<img class="ui avatar tw-align-middle" src="{{$avatarLink}}" width="24" height="24" alt="" aria-hidden="true">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not null, so why did you change it? This should not affect the width.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried it?

image

{{else if $.IsMirror}}
{{svg "octicon-mirror" 24}}
{{else if $.IsFork}}
Expand Down
36 changes: 16 additions & 20 deletions web_src/js/modules/fomantic/dropdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,26 @@ export function initAriaDropdownPatch() {
}

// the patched `$.fn.dropdown` function, it passes the arguments to Fomantic's `$.fn.dropdown` function, and:
// * it does the one-time attaching on the first call
// * it delegates the `onLabelCreate` to the patched `onLabelCreate` to add necessary aria attributes
// * it does the one-time element event attaching on the first call
// * it delegates the module internal functions like `onLabelCreate` to the patched functions to add more features.
function ariaDropdownFn(this: any, ...args: Parameters<FomanticInitFunction>) {
const ret = fomanticDropdownFn.apply(this, args);

// if the `$().dropdown()` call is without arguments, or it has non-string (object) argument,
// it means that this call will reset the dropdown internal settings, then we need to re-delegate the callbacks.
const needDelegate = (!args.length || typeof args[0] !== 'string');
for (const el of this) {
if (!el[ariaPatchKey]) {
attachInit(el);
// the elements don't belong to the dropdown "module" and won't be reset
// so we only need to initialize them once.
attachInitElements(el);
}
if (needDelegate) {
delegateOne($(el));

// if the `$().dropdown()` call is without arguments, or it has non-string (object) argument,
// it means that such call will reset the dropdown "module" including internal settings,
// then we need to re-delegate the callbacks.
const $dropdown = $(el);
const dropdownModule = $dropdown.data('module-dropdown');
if (!dropdownModule.giteaDelegated) {
dropdownModule.giteaDelegated = true;
delegateDropdownModule($dropdown);
}
}
return ret;
Expand Down Expand Up @@ -68,18 +74,9 @@ function processMenuItems($dropdown: any, dropdownCall: any) {
}

// delegate the dropdown's template functions and callback functions to add aria attributes.
function delegateOne($dropdown: any) {
function delegateDropdownModule($dropdown: any) {
const dropdownCall = fomanticDropdownFn.bind($dropdown);

// If there is a "search input" in the "menu", Fomantic will only "focus the input" but not "toggle the menu" when the "dropdown icon" is clicked.
// Actually, Fomantic UI doesn't support such layout/usage. It needs to patch the "focusSearch" / "blurSearch" functions to make sure it toggles the menu.
const oldFocusSearch = dropdownCall('internal', 'focusSearch');
const oldBlurSearch = dropdownCall('internal', 'blurSearch');
// * If the "dropdown icon" is clicked, Fomantic calls "focusSearch", so show the menu
dropdownCall('internal', 'focusSearch', function (this: any) { dropdownCall('show'); oldFocusSearch.call(this) });
// * If the "dropdown icon" is clicked again when the menu is visible, Fomantic calls "blurSearch", so hide the menu
dropdownCall('internal', 'blurSearch', function (this: any) { oldBlurSearch.call(this); dropdownCall('hide') });

const oldFilterItems = dropdownCall('internal', 'filterItems');
dropdownCall('internal', 'filterItems', function (this: any, ...args: any[]) {
oldFilterItems.call(this, ...args);
Expand Down Expand Up @@ -163,9 +160,8 @@ function attachStaticElements(dropdown: HTMLElement, focusable: HTMLElement, men
}
}

function attachInit(dropdown: HTMLElement) {
function attachInitElements(dropdown: HTMLElement) {
(dropdown as any)[ariaPatchKey] = {};
if (dropdown.classList.contains('custom')) return;

// Dropdown has 2 different focusing behaviors
// * with search input: the input is focused, and it works with aria-activedescendant pointing another sibling element.
Expand Down
4 changes: 3 additions & 1 deletion web_src/js/modules/observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@ function callGlobalInitFunc(el: HTMLElement) {
const func = globalInitFuncs[initFunc];
if (!func) throw new Error(`Global init function "${initFunc}" not found`);

// when an element node is removed and added again, it should not be re-initialized again.
type GiteaGlobalInitElement = Partial<HTMLElement> & {_giteaGlobalInited: boolean};
if ((el as GiteaGlobalInitElement)._giteaGlobalInited) throw new Error(`Global init function "${initFunc}" already executed`);
if ((el as GiteaGlobalInitElement)._giteaGlobalInited) return;
(el as GiteaGlobalInitElement)._giteaGlobalInited = true;

func(el);
}

Expand Down