issue: https://linear.app/comm/issue/ENG-2739/move-navigationpanel-from-the-sidebar-to-a-top-bar
When AppSwitcher (that uses NavigationPanel underneath) is used in the Topbar it'll need to be horizontal. The tabs will also have to display differently. But the SettingSwitcher (that also uses NavigationPanel underneath) is supposed to stay as it is.
Details
Pass horizontal={true} to NavigationPanel.Container in AppSwitcher and check that it displayes correctly.
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
JS is ok, deferring CSS to someone else
web/sidebar/navigation-panel.react.js | ||
---|---|---|
51 ↗ | (On Diff #21198) | I'm not sure about this one. Will this class name be changed when switching the horizontal prop? |
web/sidebar/navigation-panel.react.js | ||
---|---|---|
51 ↗ | (On Diff #21198) | (eslint says to do it, and even if it's not needed, I don't think it hurts) |
web/sidebar/navigation-panel.react.js | ||
---|---|---|
51 ↗ | (On Diff #21198) |
This is convincing enough, thanks ;)
The class itself changes, but this memo depends only on its name, which should be constant (unless it's hashed), this is why I asked initially ;)
Yes, but we shouldn't depend on the "immutability by design" of this prop - it isn't supposed to be mutated now, but it can be in the future and nothing's blocking it |
I'm not sure if this is the best solution. Usually in cases like this, we could introduce a new class e.g. horizontal and check it in selectors: we would have span.chatBadge selector and span.chatBadge.horizontal. This allows splitting the code which depends on the orientation from the code that doesn't. On the other hand current solution is still maintainable, so we can consider keeping it.
web/topbar/topbar.css | ||
---|---|---|
62–63 ↗ | (On Diff #21198) | We have some consts for it. |
web/topbar/topbar.css | ||
---|---|---|
62–63 ↗ | (On Diff #21198) | Actually we don't, but this is the only place in the code where we use these values, so @tomek and I agreed to leave it like this. Especially that I was just moving this code from somewhere else. |