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

Sidebar menu opened by tab + enter does not receive focus #7123

Open
wants to merge 1 commit into
base: dev-mail
Choose a base branch
from

Conversation

andrehgdias
Copy link
Contributor

  • trapping focus on the sidebar when it is opened
  • exporting hadleFocus() from GuiUtils

Co-authored-by: rih rih@tutao.de

Closes #7118

@andrehgdias andrehgdias force-pushed the 7118-sidebar-menu-opened-by-tabenter-does-not-receive-focus branch 2 times, most recently from 8e159c9 to 36040ee Compare June 21, 2024 14:09
- exporting hadleFocus() from GuiUtils
Co-authored-by: rih <rih@tutao.de>
@andrehgdias andrehgdias force-pushed the 7118-sidebar-menu-opened-by-tabenter-does-not-receive-focus branch from 36040ee to c80504f Compare July 5, 2024 09:33
@andrehgdias andrehgdias changed the base branch from master to dev-mail July 5, 2024 09:33
@@ -363,6 +365,9 @@ export class ViewSlider implements Component<ViewSliderAttrs> {
})
.finally(() => {
foregroundColumn.isInForeground = toForeground
if (foregroundColumn.domColumn && !toForeground) {
Copy link
Contributor

Choose a reason for hiding this comment

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

the previous attempt by @jalhubb was similar and led to some issues, I think would be good if he could check it

Copy link
Contributor

Choose a reason for hiding this comment

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

oh sorry I meant @rezbyte

@andrehgdias andrehgdias requested a review from jalhubb July 9, 2024 07:29
@jalhubb jalhubb removed their request for review July 9, 2024 14:04
@andrehgdias andrehgdias requested a review from rezbyte July 9, 2024 15:38
show() {
this.focusedBeforeShown = document.activeElement as HTMLElement
this.turnTrapFocus(true)
handleFocus(true, [".main-view"])
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be nicer to curry this rather than pass in [".main-view"] each time.

Comment on lines +25 to +30
onupdate(vnode: m.VnodeDOM<Attrs>): any {
if (vnode.dom.parentElement) {
const trapFocus = vnode.dom.parentElement.style.visibility === "visible"
handleFocus(trapFocus, ["nav", ".view-columns"])
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This couples the FolderColumnView to the ViewSlider which will be not so nice when we refactor the ViewSlider. I would also advise against giving components access to their parents if possible.

Have you tried creating an isFocused attribute then passing in (vnode.dom as HTMLElement).style.visibility === "visible" from the ViewSlider into it?

@rezbyte
Copy link
Contributor

rezbyte commented Jul 10, 2024

Opening the sidebar then resizing the window to three column layout, then back to single column view causes the sidebar to become unopenable.

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.

Sidebar menu opened by tab + enter does not receive focus
3 participants