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

Use OverlayMenu for TopBar #177

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Use OverlayMenu for TopBar #177

wants to merge 5 commits into from

Conversation

atjn
Copy link
Collaborator

@atjn atjn commented Dec 14, 2023

The top bar has previously used its own custom overlay menu, but all other components in the UI use the standardized OverlayMenu. This makes the top bar use the same overlay menu.

The checkboxes have never worked properly and are not connected to any logic. I have disabled some of the offending code by commenting it out. We can fix this later when we know how they will be used.

Copy link

netlify bot commented Dec 14, 2023

Deploy Preview for ecdar ready!

Name Link
🔨 Latest commit 0eaec54
🔍 Latest deploy log https://app.netlify.com/sites/ecdar/deploys/657b7f8d4b0bb700086c88ef
😎 Deploy Preview https://deploy-preview-177--ecdar.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@atjn
Copy link
Collaborator Author

atjn commented Dec 14, 2023

A lot of tests are failing, it might not be possible to fix this before code freeze. That is okay, we can leave this PR ready for the next group to merge.

The tests are probably related to the popover menu, so they might just magically start to pass in a few months when all playwright browsers start supporting the popover element.

@KamyaPA
Copy link
Contributor

KamyaPA commented Dec 14, 2023

A lot of tests are failing, it might not be possible to fix this before code freeze. That is okay, we can leave this PR ready for the next group to merge.

The tests are probably related to the popover menu, so they might just magically start to pass in a few months when all playwright browsers start supporting the popover element.

I have requested a review from he who wrote the topbar to begin with. I don't feel like I can approve these changes without his consent.

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.

3 participants