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

NOISS: Refactor sidebar menu #631

Merged
merged 14 commits into from
Sep 9, 2024
Merged

NOISS: Refactor sidebar menu #631

merged 14 commits into from
Sep 9, 2024

Conversation

kjporter
Copy link
Contributor

This PR will:

  • Finally... gives us the collapsible menu that we deserve (submenus auto-collapse with accordion-like behavior)
  • Adds some training statistics to the sidebar

@kjporter kjporter added bug Something isn't working enhancement New feature or request labels Aug 25, 2024
@kjporter kjporter requested a review from a team as a code owner August 25, 2024 06:12
Copy link
Member

@iccowan iccowan left a comment

Choose a reason for hiding this comment

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

Code looks good...QAing

@iccowan
Copy link
Member

iccowan commented Aug 27, 2024

When on the "Training Information" page, the sidebar won't expand

Also, it was intentional to leave the sidebar expanded by default when on a page that exists in the sidebar so the user would have a queue of the page they're on and where that stands in the layout. Do we want to add this back?

@kjporter
Copy link
Contributor Author

kjporter commented Sep 1, 2024

When on the "Training Information" page, the sidebar won't expand

Also, it was intentional to leave the sidebar expanded by default when on a page that exists in the sidebar so the user would have a queue of the page they're on and where that stands in the layout. Do we want to add this back?

Thanks @iccowan - I somehow missed that. Will update this to include that as a feature. @c0repwn3r noted some issues with dark mode that also require a refactor on this, so I'll sort this out soon.

@kjporter
Copy link
Contributor Author

kjporter commented Sep 6, 2024

Just so I have this captured...
If you have time, do you think you could refactor it further to remove the code that directly sets height and min-height? The reason I ask, is that for darkmode, the current implementation of the sidebar makes darkmode impossible to implement. By overriding height and min-height, the sidebar "pushes" the element out of the way to make room for itself - this results in there being a block directly below the sidebar where you can see directly through to the background rendered behind the page by the browser. I've taken a screenshot from a custom build of Firefox where I overrode this to #000 so you can see the issue - but every modern browser uses #fff here, so in dark mode, there would be a big ugly white block that we cannot get rid of in CSS since you can't style the emptiness where there is no page, if that makes sense
It's very weird behavior and not at all what the html/css spec says should happen, but this is what every modern browser does (tested with up-to-date versions of chromium, gecko, and webkit)
image

@kjporter
Copy link
Contributor Author

kjporter commented Sep 6, 2024

Fixed sidebar so it remains open with the menu option selected on 9/6.

@kjporter
Copy link
Contributor Author

kjporter commented Sep 7, 2024

We're not going to fix the zero-space problem in this PR. #645 created to capture this as a separate issue.

@kjporter
Copy link
Contributor Author

kjporter commented Sep 8, 2024

When on the "Training Information" page, the sidebar won't expand

Also, it was intentional to leave the sidebar expanded by default when on a page that exists in the sidebar so the user would have a queue of the page they're on and where that stands in the layout. Do we want to add this back?

I corrected the issue with the training information view - it was a class name conflict

@iccowan
Copy link
Member

iccowan commented Sep 9, 2024

Before merging, just note my comment from discord: "I'm gonna go ahead and approve this, but one small CSS change that I noticed. When hovering over "ZTL Controllers", "Training", or "Administration", the cursor has the typing one instead of the finger. Once that's changed back it looks good!"

@kjporter kjporter merged commit afff286 into main Sep 9, 2024
26 checks passed
@iccowan iccowan deleted the refactor-sidebar-menu branch September 16, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants