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.

Sign up for GitHub

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

Jump to bottom

fix(NcActions): only search for items to focus on in the current menu #4846

Merged
merged 1 commit into from Nov 17, 2023
Merged

fix(NcActions): only search for items to focus on in the current menu #4846

merged 1 commit into from Nov 17, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Nov 17, 2023 •
edited

☑️ Resolves

When a mouse is moved over NcActions it searches for a NcAction<item> under the cursor by searching for the closest li element.

The closest searches up to the document root.
So when the cursor is not on an actual NcAction-<item> (on UL itself between actions) and NcActions is rendered in a container, it finds just any li ancestor. In Talk, it is a message element. It results in focusing the first focusable action in this container.

🖼️ Screenshots

🏚️ Before 🏡 After
talk-before talk-after

🚧 Tasks

  • Only search items to focus on in the current menu

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@ShGKme
fix(NcActions): only search items to focus on in the current menu
9d1930d 
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme added bug Something isn't working feature: actions Related to the actions components regression Regression of a previous working feature labels Nov 17, 2023
@ShGKme ShGKme added this to the 8.1.1 milestone Nov 17, 2023
@ShGKme ShGKme self-assigned this Nov 17, 2023
@ShGKme ShGKme requested a review from skjnldsv November 17, 2023 12:10
szaimen
szaimen approved these changes Nov 17, 2023
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

LGTM from the screen recording

@susnux susnux merged commit e2b6224 into master Nov 17, 2023
15 checks passed
@susnux susnux deleted the fix/4539/nc-actions--search-item-to-focus branch November 17, 2023 12:36
@susnux susnux mentioned this pull request Nov 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@susnux susnux susnux approved these changes

@szaimen szaimen szaimen approved these changes

@DorraJaouad DorraJaouad DorraJaouad approved these changes

@Antreesy Antreesy Antreesy approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@JuliaKirschenheuter JuliaKirschenheuter Awaiting requested review from JuliaKirschenheuter

@Pytal Pytal Awaiting requested review from Pytal

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

Assignees

@ShGKme ShGKme

Labels
bug Something isn't working feature: actions Related to the actions components regression Regression of a previous working feature
Projects
None yet
Milestone
8.2.0
Development

Successfully merging this pull request may close these issues.

Scolling in additional action menu broken when hovering horizontal lines
5 participants
@ShGKme @susnux @szaimen @DorraJaouad @Antreesy