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

Top nav bar z-index #2757

Closed
twschiller opened this issue Feb 15, 2022 · 4 comments · Fixed by #2758
Closed

Top nav bar z-index #2757

twschiller opened this issue Feb 15, 2022 · 4 comments · Fixed by #2758
Assignees
Labels
bug Something isn't working user experience Improve the user experience (UX)
Milestone

Comments

@twschiller
Copy link
Contributor

twschiller commented Feb 15, 2022

The z-index of the top nav is too high. It's above our toasts and also the modal backdrop

Fix top nav z-index relative to

  • Modal backdrop
  • Toasts

image

Related PRs:

Related Code

@twschiller twschiller added this to the 1.5.5 milestone Feb 15, 2022
@twschiller twschiller added bug Something isn't working user experience Improve the user experience (UX) labels Feb 15, 2022
@twschiller
Copy link
Contributor Author

@fregante I suspect this is a regression from the CSS Module PR?

@fregante
Copy link
Collaborator

Fix top nav z-index relative to

  • Modal backdrop

Which modal? The Services modal works correctly as its z-index is sky-high

Screen Shot 9

@twschiller
Copy link
Contributor Author

twschiller commented Feb 16, 2022

Here's the z-index craziness:

I think we made a mistake converting the ShareExtensionModal to a module. The Google File Picker comment is not relevant to the ShareExtensionModal. It comes up with the ServiceEditorModal http://github.com/pixiebrix/pixiebrix-extension/blob/dc30b02f757302c85b0120eaa80aee2f9fc661b3/src/options/pages/services/ServiceEditorModal.tsx#L54-L54

As part of #2749, we're definitely going to want define all the z-indexes as variable: e.g., a variable for backdrop, modal, toast, etc.

@fregante
Copy link
Collaborator

Here's the z-index craziness:

Ah indeed, we think we need to bump those values so they're naturally above the site header (whose z-index is defined by Bootstrap)

As part of #2749, we're definitely going to want define all the z-indexes as variable: e.g., a variable for backdrop, modal, toast, etc.

Can we use SCSS variables in JS? Because that's the issue sometimes. Or maybe we should just stop using inline CSS for zIndex (but it's not always possible)

const containerStyle: React.CSSProperties = {
zIndex: NOTIFICATIONS_Z_INDEX,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working user experience Improve the user experience (UX)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants