Skip to content
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

[4.4] Fix event target in menu.js #44019

Merged
merged 6 commits into from
Sep 23, 2024
Merged

Conversation

SniperSister
Copy link
Contributor

@SniperSister SniperSister commented Sep 4, 2024

Summary of Changes

The core menu.js dynamically adds aria-hidden=true attributes to ul tags on menu level 2:

Bildschirmfoto 2024-09-04 um 10 54 57

However, on focus and hover, these attributes aren't always set to "false" (which would be correct as the previously invisible dropdown now is possible) as the JS has a bug. It uses the target attribute of the event, which is not the element that the eventhandler was bound to (=the parent <li>) but the element that triggered the event, i.e. a nested a or span tag.

It should use currentTarget.

See msdn:
https://developer.mozilla.org/en-US/docs/Web/API/Event/currentTarget

Testing Instructions

Create a test site with a multi-level menu that uses the "default" menu layout. Make sure that the menu.js is loaded and the aria-hidden attribute is appended. Now hover over the element.

Actual result BEFORE applying this Pull Request

aria-hidden isn't updated reliably.

Expected result AFTER applying this Pull Request

aria-hidden is updated reliably.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • [x ] No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • [ x] No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.4-dev labels Sep 4, 2024
@richard67 richard67 added the bug label Sep 5, 2024
@fgsw
Copy link

fgsw commented Sep 7, 2024

I have tested this item ✅ successfully on 1dc9365


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44019.

1 similar comment
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 1dc9365


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44019.

@Quy
Copy link
Contributor

Quy commented Sep 19, 2024

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44019.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 19, 2024
@MacJoom MacJoom added this to the Joomla! 4.4.9 milestone Sep 23, 2024
@MacJoom MacJoom merged commit 6043567 into joomla:4.4-dev Sep 23, 2024
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 23, 2024
@MacJoom
Copy link
Contributor

MacJoom commented Sep 23, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.4-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants