chore: Disable implicit tools integration into drawers list #1581
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1581 +/- ##
==========================================
- Coverage 94.05% 94.05% -0.01%
==========================================
Files 649 649
Lines 17572 17569 -3
Branches 5770 5772 +2
==========================================
- Hits 16528 16524 -4
- Misses 975 976 +1
Partials 69 69
☔ View full report in Codecov by Sentry. |
just-boris
force-pushed
the
feat-disable-drawers-tools-integration
branch
from
3e069e6
to
f7f9418
Compare
just-boris
force-pushed
the
feat-disable-drawers-tools-integration
branch
from
f7f9418
to
a5c7bbd
Compare
just-boris
force-pushed
the
feat-disable-drawers-tools-integration
branch
from
a5c7bbd
to
282e351
Compare
| }); | ||
|
|
||
| test('should apply drawers treatment to the tools if at least one other drawer is provided', () => { |
There was a problem hiding this comment.
This and other tests from this file moved into runtime-drawers.test.tsx, because the tools+drawers merging feature is now only available for runtime drawers
| fireNonCancelableEvent(ownDrawers?.onChange, newDrawerId); | ||
| } | ||
| if (!toolsProps.toolsHide) { | ||
| fireNonCancelableEvent(toolsProps.onToolsChange, { open: newDrawerId === TOOLS_DRAWER_ID }); |
There was a problem hiding this comment.
Removed repeating onToolsChange handlers from other places, this is the only one where we keep it to support compatibility between tools and drawers
There was a problem hiding this comment.
Do we need to check if tools are ignored?
There was a problem hiding this comment.
We still have "multi drawer with tools mode" for runtime API, this is why the condition is here. See applyToolsDrawer function how exactly this works
| @@ -36,7 +36,7 @@ import { useUniqueId } from '../../internal/hooks/use-unique-id'; | |||
|
|
|||
| interface AppLayoutInternals extends AppLayoutProps { | |||
| activeDrawerId: string | undefined; | |||
| drawers: Array<DrawerItem>; | |||
| drawers: Array<DrawerItem> | null; | |||
There was a problem hiding this comment.
Empty array [] means that <AppLayout drawers={[]}> is used and null means that there are no drawers registered in any form. This distinction is needed to decide what we render on the right hand side
| @@ -50,7 +50,7 @@ export default function Layout({ children }: LayoutProps) { | |||
|
|
|||
| // Content gaps on the left and right are used with the minmax function in the CSS grid column definition | |||
| const hasContentGapLeft = isNavigationOpen || navigationHide; | |||
| const hasContentGapRight = drawersTriggerCount <= 0 || hasOpenDrawer; | |||
| const hasContentGapRight = drawersTriggerCount === 0 || hasOpenDrawer; | |||
There was a problem hiding this comment.
I do not think this count can ever be negative number, so decided to simplify for readability
just-boris
force-pushed
the
feat-disable-drawers-tools-integration
branch
from
282e351
to
08565a2
Compare
just-boris
force-pushed
the
feat-disable-drawers-tools-integration
branch
from
08565a2
to
d509db0
Compare
Description
Implement this proposal: ZuAXAsmOsora
Related links, issue #, if available: n/a
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.