facebookincubator / infima

A UI framework that provides websites with the minimal CSS and JS needed to get started with building a modern responsive beautiful website
https://infima.dev
MIT License
408 stars 55 forks source link

navbar shouldn't contain the navbar mobile sidebar #275

Open slorber opened 2 years ago

slorber commented 2 years ago

We should be able to set different stacking contexts (ie z-index layers) for the navbar and the mobile sidebar.

Not doing so is annoying because we want some elements to be able to cover the navbar (announcementBar shadow, skipToContent)... and yet have the mobile navbar cover those.

I gave it a try in https://github.com/facebook/docusaurus/pull/7940 to increase the announcementBar z-index so that its shadow can be seen on top of the navbar (gives a better sensation of depth and seems the right thing to do)

CleanShot 2022-10-06 at 12 41 21@2x

But this leads to issues on mobile:

CleanShot 2022-10-06 at 12 42 47@2x CleanShot 2022-10-06 at 12 43 25@2x

Due to the current inability of having 2 separate stacking contexts, I have to fix the 2 issues above by reverting my change, and removing the announcementBar shadow in favor of a bottom border.


We should refactor Infima to allow rendering the navbar and its associated mobile sidebar in 2 separate DOM trees. Ie not having classes such as .navbar.navbar-sidebar--show .navbar-sidebar like we currently have, assuming the sidebar is a child of the navbar.

@yangshun if it were me I'd remove all things related to macro-layout orchestration like fixed/sticky positioning and margins from Infima and only declare these things on the Docusaurus side. Having this in Infima creates DX friction to test the change easily locally and in PRs. I'd rather have Infima focus only on the design system elements and not how elements are displayed inside the app. Any opinion?

Note that if we want to have a Tailwind theme, it's likely to have the same mobile sidebar behavior, animation and positions. I'd rather have this CSS in @docusaurus/theme-common as shared CSS modules so that we can avoid duplicating it in each design system / theme.