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

fix(ui5-menu): restore focus to the opener #9041

Merged
merged 22 commits into from
Jul 3, 2024

Conversation

unazko
Copy link
Contributor

@unazko unazko commented May 22, 2024

  • The focus is restored over the opener element after root menu close.
  • Redundant icon dependency is removed from the menu.
  • Escape keyboard key press closes all dialogs on mobile device.
  • Sub-menu could not be opened from a disabled ui5-menu-item.
  • The leftover start-section property usage is replaced with the ui5-menu-separator component usage.

Fixes: #9317

- The focus is restored over the opener element
after root menu close.
- Redudant icon dependency is removed from the menu.
@unazko unazko requested a review from TeodorTaushanov May 22, 2024 12:25
@unazko
Copy link
Contributor Author

unazko commented Jun 5, 2024

The prevent-focus-restore attribute is now removed from the menu template as a better approach for restoring the focus to the opener button pointed out by @TeodorTaushanov.

@unazko unazko marked this pull request as ready for review June 5, 2024 09:13
@unazko unazko requested a review from tsanislavgatev June 5, 2024 09:15
@unazko unazko requested a review from nnaydenow June 27, 2024 13:12
@nnaydenow
Copy link
Contributor

Test looks good to me but I haven’t review the rest of PR.

tsanislavgatev
tsanislavgatev previously approved these changes Jul 2, 2024
@tsanislavgatev
Copy link
Contributor

Merge main into your branch to get the change that makes the tests stable.

@nnaydenow
Copy link
Contributor

Test is failing because following code doesn't work and the focus is not restored to the button when the menu is closed.

	<ui5-button id="test">Open Menu</ui5-button>
	<ui5-menu>
		<ui5-menu-item text="New File">
		</ui5-menu-item>
	</ui5-menu>
	<script>
			const button = document.querySelector("#test")

			button.addEventListener("click", (e) => {
				const menu = document.querySelector("[ui5-menu]")
				menu.open = true;
				menu.opener = e.target;
			})
	</script>

@tsanislavgatev tsanislavgatev dismissed their stale review July 3, 2024 06:34

Tested manually with latest and the fix does not work as expected in the github issue.

Copy link
Contributor

@tsanislavgatev tsanislavgatev left a comment

Choose a reason for hiding this comment

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

After discussion and additional test we've came to the conclusion that the latest version works as expected and described in the issue.

@unazko unazko merged commit 0ada944 into SAP:main Jul 3, 2024
10 checks passed
@nnaydenow nnaydenow mentioned this pull request Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ui5-menu]: submenu is open from disabled item
4 participants