issue: https://linear.app/comm/issue/ENG-2739/move-navigationpanel-from-the-sidebar-to-a-top-bar
The AppSwitcher is now displayed in the topbar. The SettingsSwitcher is displayed as it used to, but I moved it out of the sidebar because of how we use the grid.
Our grid: https://github.com/CommE2E/comm/blob/master/web/style.css#L54
So the SettingsSwitcher has to be a part of app grid area, or the grid layout has to be dynamic.
Changing the way we use the grid - or not using the grid - can be done, but it seems like a big side quest to my community drawer task. So for now I think this is a good enough solution.
It is also consistent between the AppSwitcher and SettingsSwitcher, so I think it's not necessarily bad.
Details
Run web app, check that the SettingSwitcher and AppSwitcher display and work correctly.
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
web/calendar/filter-panel.css | ||
---|---|---|
4 ↗ | (On Diff #21242) | This makes filters panel in calendar tab display under my topbar, and not partially over it |
web/app.react.js | ||
---|---|---|
227–262 ↗ | (On Diff #21242) | I know you just moved most of it to a separate function, but maybe it's a good place to do some refactoring? I'm not a fan of such an imperative approach (empty let, nested if-else etc.). But it's only my subjective opinion, so up to you. |
web/app.react.js | ||
---|---|---|
227–262 ↗ | (On Diff #21242) | This is how it's done in most places in the code base. I'm not a huge fun of this approach either, but don't see the point of refactoring just one out of thousands places like this |
web/calendar/filter-panel.css | ||
---|---|---|
4 ↗ | (On Diff #21242) | It would be a lot better if this component was a part of basic layout instead of making its position fixed. It should be possible to position it using e.g. flexbox, but it definitely isn't worth doing now. |