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

FEATURE: Force admin sidebar for all admins in admin_sidebar_enabled_groups and handle legacy "hamburger dropdown" in this mode #26899

Merged
merged 7 commits into from May 13, 2024

Conversation

martin-brennan
Copy link
Contributor

@martin-brennan martin-brennan commented May 7, 2024

Some sites are still on the legacy "hamburger dropdown"
navigation_menu setting. In this case to avoid confusion,
we want to show both the sidebar icon and the header dropdown
hamburger when visiting the admin portal. Otherwise, the
hamburger switches sides from right to left for admins
and takes on different behaviour.

The hamburger in this case only shows the main panel, not
other sidebar panels like the admin one.

2024-05-10_14-18
2024-05-10_14-18_1

Some sites are still on the legacy "hamburger dropdown"
navigation_menu setting. In this case to avoid confusion,
we want to show both the sidebar icon and the header dropdown
hamburger when visiting the admin portal. Otherwise, the
hamburger switches sides from right to left for admins
and takes on different behaviour.
@martin-brennan martin-brennan marked this pull request as ready for review May 10, 2024 04:16
@martin-brennan martin-brennan changed the title WIP: Show hamburger dropdown when forcing admin sidebar FEATURE: Show hamburger dropdown when forcing admin sidebar May 10, 2024
Copy link
Contributor

@Drenmi Drenmi left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Part of me would like to find a shorter name for adminSidebarAllowedWithLegacyNavigationMenu, but I don't think it's critical. 🙂

@martin-brennan
Copy link
Contributor Author

Part of me would like to find a shorter name for adminSidebarAllowedWithLegacyNavigationMenu, but I don't think it's critical. 🙂

Yeah I think it's fine mainly because it signals that, yes, this is kind of a screwed up thing to have to support 😂

@martin-brennan martin-brennan changed the title FEATURE: Show hamburger dropdown when forcing admin sidebar FEATURE: Force admin sidebar for all admins in admin_sidebar_enabled_groups and handle legacy "hamburger dropdown" in this mode May 13, 2024
@martin-brennan martin-brennan merged commit 9bcbfbb into main May 13, 2024
16 checks passed
@martin-brennan martin-brennan deleted the feature/admin-sidebar-icon-header-dropdown branch May 13, 2024 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants