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

[NcAppSidebar] add open state #5335

Closed
ShGKme opened this issue Mar 1, 2024 · 3 comments · Fixed by #5584
Closed

[NcAppSidebar] add open state #5335

ShGKme opened this issue Mar 1, 2024 · 3 comments · Fixed by #5584
Labels
enhancement New feature or request feature: app-sidebar Related to the app-sidebar component

Comments

@ShGKme
Copy link
Contributor

ShGKme commented Mar 1, 2024

Currently NcAppSidebar doesn't have changeable open state. If it is rendered, it considers itself to be open. The only way to close the sidebar is basically to not render it like:

<NcAppSidebar v-if="open" />

But some apps want to keep NcAppSidebar instance even if it is not displayed.

<NcAppSidebar v-show="open" />

This breaks mobile devises, because NcAppSidebar consider itself open and enables a focus trap.

A solution could be to add open prop allowing to show/hide the sidebar keeping the component and the element in the DOM like NcAppNavigation does.

In this case, we can even render the toggle button like in Talk.

image

@ShGKme ShGKme added enhancement New feature or request feature: app-sidebar Related to the app-sidebar component labels Mar 1, 2024
@Antreesy
Copy link
Contributor

Antreesy commented Mar 8, 2024

I'm afraid, same happens with NcAppNavigation too:

When it's in mobile state, focus trap is attached to toggle, and thus any open dialog would be inaccessible from keyboard (not saying that I couldn't type in TextField)
image

Any known PR, where this regression could come from? I could move it to a new bug issue

@ShGKme
Copy link
Contributor Author

ShGKme commented Mar 8, 2024

Hmm, with dialog it should work. Dialogs are supposed to have a focus trap, so it should stack correctly

@ShGKme
Copy link
Contributor Author

ShGKme commented Mar 8, 2024

It is different with Sidebar. The sidebar issue in Talk is that Sidebar is actually open but visually hidden.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature: app-sidebar Related to the app-sidebar component
Projects
None yet
2 participants