Fix: Navbar: mobile dropdown menu not opening #94702

Francesco Bellini · 2024-09-18T18:42:05+02:00

Francesco Bellini commented

2024-09-18 18:42:05 +02:00

Hi there! First time trying to contribute to an existing blender project.

So, lately i'm using the Extensions Platform a lot.

I noticed that opening the navbar dropdown menu (Extensions button, to be clear), from mobile, was requiring some kind of sniper skills, to open properly.

Clicking the text "Extensions" or the little arrow, wasn't working as expected.

So i forked the extensions-website, just to realize that the code i was interested to, was here, in the v2 branch, which is currently linked as a submodule of the extensions-website.

What was happening?

The problem was occurring when clicking on the "Extensions" text, or the little arrow icon.

Basically, clicking the text "Extensions" was correctly triggering the parent button, and the relative code to toggle the dropdown was running as intended (dropdownToggle()), but after it was finished, the click event was propagated to the <strong> text "Extensions".

At this point the problem arise, because another listener (the one intended to close dropdowns by clicking elsewhere; see code) was kicking in:
Since the <strong> tag was not recognised as a valid trigger for any dropdown menu, the listener was closing the dropdown right-away, as if user were clicking elsewhere with the menu opened.

To be more clear, clicking here wasn't working:

While clicking, for example, here, it's working:

A fairly simple event.stopPropagation() solve the problem, by running only the expected dropdownToggle, without also running the closing listener, by mistake.

I understand this is an asset repo, so i'm not sure if this is (or not) used also elsewhere, but i don't think it should affect in a bad way eventual other uses of this method, as long as the method is only used by listeners, receiving the event parameter.

Hope you'll appreciate this very small contribution!

Hi there! First time trying to contribute to an existing blender project. --- So, lately i'm using the Extensions Platform a lot. I noticed that opening the navbar dropdown menu (Extensions button, to be clear), **from mobile**, was requiring some kind of sniper skills, to open properly. ![immagine](/attachments/00755782-1016-4d89-9dd9-71fe8209684d) _Clicking the text "Extensions" or the little arrow, wasn't working as expected._ _So i forked the extensions-website, just to realize that the code i was interested to, was here, in the v2 branch, which is currently linked as a submodule of the extensions-website._ ### What was happening? The problem was occurring when clicking on the "Extensions" text, or the little arrow icon. Basically, clicking the text "Extensions" was correctly triggering the parent button, and the relative code to toggle the dropdown was running as intended (`dropdownToggle()`), but after it was finished, the click event was propagated to the `<strong>` text "Extensions". At this point the problem arise, because another listener (_the one intended to close dropdowns by clicking elsewhere; [see code](https://projects.blender.org/Francesco-Bellini/web-assets/src/commit/1a6958966901ca8129a72e0b5c5cbb7f57a1700a/src/scripts/tutti/10_navbar.js#L81)_) was kicking in: Since the `<strong>` tag was not recognised as a valid trigger for any dropdown menu, the listener was closing the dropdown right-away, as if user were clicking elsewhere with the menu opened. --- To be more clear, clicking here wasn't working: ![immagine](/attachments/691eab30-6c72-4710-a9d3-0f45a124ce96) While clicking, for example, here, it's working: ![immagine](/attachments/7ed3b1d1-e38f-4fde-a8de-8ec6f3bb467d) --- A fairly simple `event.stopPropagation()` solve the problem, by running only the expected `dropdownToggle`, without also running the closing listener, by mistake. --- _I understand this is an asset repo, so i'm not sure if this is (or not) used also elsewhere, but i don't think it should affect in a bad way eventual other uses of this method, as long as the method is only used by listeners, receiving the event parameter._ Hope you'll appreciate this very small contribution!

immagine.png

4.8 KiB

immagine.png

4.5 KiB

immagine.png

4.5 KiB

👍 1

Francesco Bellini added 1 commit 2024-09-18 18:42:06 +02:00

fix: stop propagation on dropdown trigger (avoid auto-close) 1a69589669

Pablo Vazquez changed title from ~~Fix Extensions Platform navbar dropdown menu opening (for mobile only)~~ to Fix: Navbar: mobile dropdown menu not opening

2024-09-19 11:23:50 +02:00

Pablo Vazquez commented

2024-09-19 11:32:04 +02:00

Ciao Francesco! Thanks for taking the time to investigate and provide such a detailed solution.

I can confirm the issue. Another potential solution (less elegant) would be to use CSS to make the elements inside the dropdown ignore pointer-events. But that'd be a hack and not the proper way.

Because this is done on dropdownToggle it affects every dropdown menu, so other cases should be tested.
I will let our front-end dev @martonlente review this.

Note: I renamed the PR to be more general because it affects every project using web-assets.

Ciao Francesco! Thanks for taking the time to investigate and provide such a detailed solution. I can confirm the issue. Another potential solution (less elegant) would be to use CSS to make the elements inside the dropdown ignore `pointer-events`. But that'd be a hack and not the proper way. Because this is done on `dropdownToggle` it affects every dropdown menu, so other cases should be tested. I will let our front-end dev @martonlente review this. Note: I renamed the PR to be more general because it affects [every project using web-assets](https://projects.blender.org/infrastructure/web-assets/wiki/Version-tracker.md).

👍 2

Márton Lente commented

2024-09-19 19:41:13 +02:00

Hi @Francesco-Bellini ,

Thank you for your thorough description, and the nice fix! I found it to indeed resolve the issue, and reliably work without negative side-effects. I'll create a patch with the fix, which will get into projects that use the repo soon.

@pablovazquez other cases look good to me. As functions from navbar.js can sometimes be used outside of the navbar, we might revisit its naming.

Hi @Francesco-Bellini , Thank you for your thorough description, and the nice fix! I found it to indeed resolve the issue, and reliably work without negative side-effects. I'll create a patch with the fix, which will get into projects that use the repo soon. @pablovazquez other cases look good to me. As functions from `navbar.js` can sometimes be used outside of the navbar, we might revisit its naming.

👍 2

Márton Lente merged commit 8cf3d6285b into v2

2024-09-19 19:44:46 +02:00

Márton Lente referenced this issue from a commit

2024-09-19 19:44:49 +02:00

Fix: Navbar mobile dropdown menu not opening

Francesco Bellini commented

2024-09-19 20:06:33 +02:00

Great, thanks!
It's a very small first commit, nevertheless feels good to give my first contribution in an official Blender project!

Thank you both for your fast responses and reviews! It's apprecieated.

Great, thanks! It's a very small first commit, nevertheless feels good to give my first contribution in an official Blender project! Thank you both for your fast responses and reviews! It's apprecieated.

❤️ 2

Pablo Vazquez commented

2024-09-24 13:02:19 +02:00

It's a very small first commit, nevertheless feels good to give my first contribution in an official Blender project!

Every bit helps. Even though web-development is/should be more accessible than Blender development, we don't really get many contributions from the community for web projects, and they are the backbone of Blender's growth. So, thank you!

> It's a very small first commit, nevertheless feels good to give my first contribution in an official Blender project! Every bit helps. Even though web-development is/should be more accessible than Blender development, we don't really get many contributions from the community for web projects, and they are the backbone of Blender's growth. So, thank you!

👍 1 ❤️ 1

Sign in to join this conversation.

No reviewers