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

Focus first action after opening actions menu #3151

Closed
wants to merge 1 commit into from

Conversation

raimund-schluessler
Copy link
Contributor

This is a dirty hack to solve #3053 and get keyboard navigation in the Actions menu to work without pressing tab once.

I have honestly no idea why a timeout is necessary here. All my attempts of using nextTick or listening for floating-vue events (even with #3149 included) did not succeed. The menu is open, and the button is there and defined, but still focus() does nothing when called without a timeout directly after the menu was open. The timeout duration is chosen arbitrarily, a timeout of 1 ms works for me as well (0 does not).

Maybe this inspires someone to find the correct solution, if not, I think the PR at least does not make it worse 🙈

@raimund-schluessler
Copy link
Contributor Author

raimund-schluessler commented Sep 1, 2022

This might be a problem of floating-vue: Akryum/floating-vue#872 actually, which automatically gives the focus to the popover. One can actually see, that the first action has focus for a short amount of time, and then loses it again. I don't see a way to disable this behavior, so the timeout might be our best option for now.

@raimund-schluessler raimund-schluessler added bug Something isn't working 3. to review Waiting for reviews feature: actions Related to the actions components 4. to release Ready to be released and/or waiting for tests to finish regression Regression of a previous working feature labels Sep 1, 2022
@raimund-schluessler raimund-schluessler added this to the 6.0.0 milestone Sep 1, 2022
@raimund-schluessler
Copy link
Contributor Author

/backport to stable6

src/components/NcActions/NcActions.vue Outdated Show resolved Hide resolved
@vinicius73
Copy link
Contributor

This might be a problem of floating-vue: Akryum/floating-vue#872 actually, which automatically gives the focus to the popover. One can actually see, that the first action has focus for a short amount of time, and then loses it again. I don't see a way to disable this behavior, so the timeout might be our best option for now.

I have a problem indirectly related to this.

I didn't find the root cause, but I guess it is related to multiple focus traps activate at the same time.

focusElement.focus() is causing recursive calls of focus trap.

image

image

@raimund-schluessler
Copy link
Contributor Author

I have a problem indirectly related to this.

I created a new issue for this #3154.

@raimund-schluessler raimund-schluessler removed the 4. to release Ready to be released and/or waiting for tests to finish label Sep 2, 2022
@jotoeri jotoeri modified the milestones: 6.0.0, 6.0.1 Oct 12, 2022
@skjnldsv
Copy link
Contributor

skjnldsv commented Nov 17, 2022

@skjnldsv
Copy link
Contributor

It's fixed without this PR :)

@skjnldsv
Copy link
Contributor

It's fixed without this PR :)

AH, not really.
But you don't need the no-auto-focus. It's disabled by default 🚀

@skjnldsv
Copy link
Contributor

So, it is being focused, but pressing tab focus the first element again 🤔
I guess there might be some interference. This is a very very minor issue though

@raimund-schluessler
Copy link
Contributor Author

It's fixed without this PR :)

AH, not really. But you don't need the no-auto-focus. It's disabled by default 🚀

Indeed, it works as it is now with Akryum/floating-vue#894. I just checked before touching this PR again, and it didn't, but that was probably a caching issue, I guess.

So, it is being focused, but pressing tab focus the first element again 🤔

What do you mean? After opening the Actions menu, keydown or tab moves to the second action, as it was before we moved to floating-vue. This is how I would expect it to work.

I guess there might be some interference. This is a very very minor issue though

I was just checking my stale PRs in this repo, and thought it would be an easy fix. Which it turned out to be, since it was in fact working already. Hence, closing the PR.

@raimund-schluessler raimund-schluessler deleted the fix/3053/actions-keyboard branch November 18, 2022 12:10
@skjnldsv
Copy link
Contributor

Thanks @raimund-schluessler ! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews backport-request bug Something isn't working feature: actions Related to the actions components regression Regression of a previous working feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants