Add fomantic-dropdown webcomponent

[McRaeAlex]

Closes #33532

Using a vanilla web component add the correct event listener for handling dropdowns.

Screen.Recording.2025-02-25.at.1.38.41.PM.mov

[delvh]

Pity to see more jQuery usage, but I know that there is no way around that for now

[silverwind]

I see the size of the webpack chunk did not change, which means webpack is not bundling anything extra into that chunk:

asset js/webcomponents.js 87.9 KiB

So I guess this is more like a convenience-only thing right now.

[lunny]

What's the long-term plan for the new component? Will it be applied to all other menus? Maybe it's a placeholder for a rewritten, so that we can have a name without related to fomantic-ui.

[McRaeAlex]

@lunny @silverwind Right now this is a small wrapper to ensure event listeners are correctly added. This enabled the use of dropdowns in both vuejs and when dynamically added to pages without manually adding the $(el).dropdown() call.

Longer term I would like to work with you to figure out a roadmap for #29849 which would eventually replace this code. For example I tried out shoelace.style for this and it seems promising but limiting in the ability to style it.

[wxiaoguang]

Sorry, we can't do that.

The dropdown is quite special, we already have a complex system to init various dropdowns correctly. This approach

    if (window.jQuery) {
      window.$(this).dropdown();
    }

will definitely break.

[wxiaoguang]

The real problem is in onShowMoreFiles, it forgets to init the dropdowns (and maybe other elements).

The proper fix is to init necessary elements in onShowMoreFiles, but not using the fragile webcomopnent.

[wxiaoguang]

Need to call init functions instead of using webcomponent

For example: initCompReactionSelector and others.

Webcomponent won't help too much for our case. At the moment, the best approach I can think of is: #33498 (comment)

---

<button data-global-click="show-panel"></button>
<a data-global-click="link-acton"></a>
<div class="dropdown" data-global-init="initMyDropdown"></div>
<div class="editor" data-global-init="initComboMarkdownDropdown"></div>
<input name="avatar" data-global-init="initAvatarUploaderWithCropper">

Then we could resolve many longstanding problems:

1. avoid many duplicate "init" calls
2. incorrect init & double init (especially when use htmx or partial reloading)

[McRaeAlex]

Sorry, we can't do that.

The dropdown is quite special, we already have a complex system to init various dropdowns correctly. This approach

    if (window.jQuery) {
      window.$(this).dropdown();
    }

will definitely break.

Can you explain why you believe this code is fragile?

Need to call init functions instead of using webcomponent

For example: initCompReactionSelector and others.

I don't see why #33498 (comment) is better than wrapping that init call into a web component for example <combo-markdown-dropdown class="editor" />.

[wxiaoguang]

Can you explain why you believe this code is fragile?

Because it is not able to call "init" functions correctly, see the comment below. Have you called initCompReactionSelector? Without it, the comment-reaction-button (which is in ui dropdown select-reaction) won't work.

I don't see why #33498 (comment) is better than wrapping that init call into a web component for example <combo-markdown-dropdown class="editor" />.

See the comment below and the explanation above.

[wxiaoguang]

By the way, a lot of frontend refactorings, including rewriting Vue (#17301, #23405, #23421, ...), rewriting the init functions (#17315), rewriting the "combo markdown editor" (#23876), rewriting the "dropdown" a lot, are all done by me, web components were also introduced by me (since #22861), there are too many details in them, and meanwhile there are still too many problems which are caused by the tricky legacy framework mechanisms from various old code.

---

The experience tells me that we'd like to refactor or introduce a new approach, I think it's better to collect all use cases for the component and design a complete solution. Otherwise something would be missing or broken. And the new approach must work well for most cases and shouldn't cause new conflicts.

---

The problem of <fomantic-dropdown>:

1. It causes duplicate init: some dropdown elements will be initialized again after fetch (ajax) loading. You will initialize them twice by this uncontrollable behavior.
2. It is unable to initialize dropdowns with custom code (see the initCompReactionSelector example). The custom init behavior is not possible to be supported by a web component.

So overall, I do not think it could really work.

[silverwind]

I also don't really like this, it doesn't really achieve anything useful imho.

I think effort would be better put into finding a replacement for the fomantic dropdowns, and finally fomantic-ui and jQuery through that.

[McRaeAlex]

I will stop working on this for now if we still feel the general approach isn't worth the effort, I don't agree but will yield.

For now it seems the correct solution is to add code in "show more" to search for .dropdown and add the correct event listener.

Given #29849 is not resolved yet, I will put further explorations in the issue.

Below are some notes on why I still feel this approach is a path forward but understand this may not sway you.

---

It causes duplicate init: some dropdown elements will be initialized again after fetch (ajax) loading. You will initialize them twice by this uncontrollable behavior.

This only occurs if we don't remove the global initializer as well. See PR below.

It is unable to initialize dropdowns with custom code (see the initCompReactionSelector example). The custom init behavior is not possible to be supported by a web component.

@wxiaoguang here is a branch which implements reaction-button main...McRaeAlex:gitea:am/reaction using the same style as the current PR.

I believe doing it this way ensures that users of reaction button don't have to remember to explicitly call initCompReactionSelector and $().dropdown.

If we still wanted to keep the initCompReactionSelector for some reason we could also just add dropdown behaviour using the code in this PR which will still prevent a call to dropdown // $conversation.find('.dropdown').dropdown(); https://github.com/McRaeAlex/gitea/blob/1548cf32d22a99034211c3962f012905e2d0e099/web_src/js/features/repo-diff.ts#L120

I also don't really like this, it doesn't really achieve anything useful imho.

I think effort would be better put into finding a replacement for the fomantic dropdowns, and finally fomantic-ui and jQuery through that.

The experience tells me that we'd like to refactor or introduce a new approach, I think it's better to collect all use cases for the component and design a complete solution. Otherwise something would be missing or broken. And the new approach must work well for most cases and shouldn't cause new conflicts.

I see this as a migration path from jquery + fomantic -> something new. By moving each logical component to a web component which encapsulates the jquery code we can replace each component one by one with a new approach. In some cases, like the reaction button that may involve wrapping the new approaches dropdown with some extra behaviour.

Right now (I, personally) don't have a good understanding of all the components that will be required from a new approach so can not propose anything beyond simple dropdowns. It may be worth aggregating all the components we need in the issue #29849.

[wxiaoguang]

@wxiaoguang here is a branch which implements reaction-button main...McRaeAlex:gitea:am/reaction using the same style as the current PR.

I am pretty sure it is not a feasible approach for current framework.

1. By your approach, we need to move almost all code from "index.js" to "webcomponent.js" step by step
   • It's not really a blocker, because we could use something like <script defer="defer"> to move the script loading to page head (just like github), but at the moment, it's not feasible before we make the framework overall right first.
2. Still, the if (window.jQuery) window.$(this).dropdown(); code is quite fragile and difficult to maintain, there is no clear call path to show readers when the component will be initialized or whether it would be be initialized twice. Every developer would spend much time on thinking and asking: why if (window.jQuery)? How is it initialized?
3. initCompReactionSelector is only a special legacy case found by your PR, there are more cases which are more complex to handle
   • For example: the complex usages like "repo-issue-content.ts" and others, would you like to refactor that part to webcomponent by your approach?
     • If yes, we need to design a complete solution before really making code change.
     • If no, I do not think we should keep <fomantic-dropdown> and <div class="ui dropdown"> at the same, it would only cause more problems.

I believe doing it this way ensures that users of reaction button don't have to remember to explicitly call initCompReactionSelector and $().dropdown.

I agree, we need to avoid keeping manually write these "init" calls again and again, my proposal (#33723 (review)) could also remove these explicit calls, and doesn't need if (window.jQuery) trick.

[wxiaoguang]

I believe doing it this way ensures that users of reaction button don't have to remember to explicitly call initCompReactionSelector and $().dropdown.

And by the way, due to the legacy framework problem, initCompReactionSelector is not only for "dropdown" elements.

That's why you still need to manually call it at the moment, but not move it to the webcomponent.

[silverwind]

we need to move almost all code from "index.js" to "webcomponent.js"

Keep in mind that by doing that, the webcomponent chunk will grow and if I understand correctly, this chunk synchonously blocks page rendering of the webcomponent elements on the page, so I'd prefer to keep its size down.

[McRaeAlex]

The jquery check doesn't need to exist if we load the web components after jquery is defined or bundle it with the web components.

I will look into the data-global-init further.

we need to move almost all code from "index.js" to "webcomponent.js"

Keep in mind that by doing that, the webcomponent chunk will grow and if I understand correctly, this chunk synchonously blocks page rendering of the webcomponent elements on the page, so I'd prefer to keep its size down.

I don't believe this to be true generally. web.dev article This means we could load web components as we load js now. Undefined web components are progressively enhanced as they are treated as divs (HTMLElement) until the web component is defined.

[silverwind]

I don't believe this to be true generally. web.dev article This means we could load web components as we load js now. Undefined web components are progressively enhanced as they are treated as divs (HTMLElement) until the web component is defined.

Interesting. So this means a <my-element>foo</my-element> paints as <div>foo</div> until the JS kicks in. Good to know. Our webcomponents chunk loads so fast currently that I wasn't aware of this graceful enhancement mechanism.

[McRaeAlex]

1. By your approach, we need to move almost all code from "index.js" to "webcomponent.js" step by step
   • It's not really a blocker, because we could use something like <script defer="defer"> to move the script loading to page head (just like github), but at the moment, it's not feasible before we make the framework overall right first.

This is true, it would be quite a bit of work to do so. However since each component in isolation is fairly easy to convert. Since we basically do the same thing in index.ts each time.

Query all elements matching some selector -> run an init function using that element at the root in order to progressively enhance it. Instead of having to query each element and loop through them to init we rely on the browser calling the connectedCallback.

2. Still, the if (window.jQuery) window.$(this).dropdown(); code is quite fragile and difficult to maintain, there is no clear call path to show readers when the component will be initialized or whether it would be be initialized twice. Every developer would spend much time on thinking and asking: why if (window.jQuery)? How is it initialized?

We can either load web components after jquery is initialized or bundle jquery with web components to avoid the if statement.

3. initCompReactionSelector is only a special legacy case found by your PR, there are more cases which are more complex to handle

   • For example: the complex usages like "repo-issue-content.ts" and others, would you like to refactor that part to webcomponent by your approach?

     • If yes, we need to design a complete solution before really making code change.
     • If no, I do not think we should keep <fomantic-dropdown> and <div class="ui dropdown"> at the same, it would only cause more problems.

I have gone through repo-issue-content.ts and I believe we would convert this to one or more web components. So yes.

However, its unclear to me what you mean by a complete solution.

I believe doing it this way ensures that users of reaction button don't have to remember to explicitly call initCompReactionSelector and $().dropdown.

I agree, we need to avoid keeping manually write these "init" calls again and again, my proposal (#33723 (review)) could also remove these explicit calls, and doesn't need if (window.jQuery) trick.

Something thats unclear about the proposed data-global-init to me, is who is responsible for actually ensuring that its called? When I dynamically add nodes to a page am I responsible for calling something similar to htmx.process to ensure all the initializers run?

With web components, as long as the component is defined its the browsers job to call connectedCallback.

[McRaeAlex]

Just something to note: I think the conversation has expanded in scope quite a bit from getting dropdowns working on dynamically added pages to more generally "how do we handle progressively enhancing the frontend".

It might be worth moving the conversation to #5937 with different proposals on how to move forward.

[wxiaoguang]

It might be worth moving the conversation to #5937 with different proposals on how to move forward.

Actually #5937 is stale for long time and even more off-topic/out-dated. I do not see any real UI improvement progress is really from it (for recent years) .....

---

FYI: Refactor repo-diff.ts #33746 , a new missing "init" call is found (initRepoDiffFileViewToggle), which is not suitable to be handled by web component IMO.

[wxiaoguang]

This PR: "Fix dynamic content loading init problem #33748" fixes all the FIXME problems:

// FIXME: here the init calls are incomplete: at least it misses dropdown & initCompReactionSelector & initRepoDiffFileViewToggle

And we do not need to call any "dropdown()" explicitly.

[McRaeAlex]

Closed by #33748