-
Notifications
You must be signed in to change notification settings - Fork 172
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
Grids: presetve repeat from inspector changes #6388
Conversation
#14293 Bundle Size — 62.65MiB (-0.14%).c769e59(current) vs 916e8b9 master#14274(baseline) Warning Bundle contains 70 duplicate packages – View duplicate packages Bundle metrics
|
Current #14293 |
Baseline #14274 |
|
---|---|---|
Initial JS | 45.78MiB (-0.19% ) |
45.86MiB |
Initial CSS | 0B |
0B |
Cache Invalidation | 22.47% |
22.36% |
Chunks | 30 |
30 |
Assets | 33 |
33 |
Modules | 4382 (-0.09% ) |
4386 |
Duplicate Modules | 520 (-0.76% ) |
524 |
Duplicate Code | 31.62% (-0.19% ) |
31.68% |
Packages | 472 |
472 |
Duplicate Packages | 70 |
70 |
Bundle size by type 2 changes
2 improvements
Current #14293 |
Baseline #14274 |
|
---|---|---|
JS | 62.64MiB (-0.14% ) |
62.73MiB |
HTML | 11.12KiB (-0.32% ) |
11.16KiB |
Bundle analysis report Branch feat/grid-resize-keep-repeat-ins... Project dashboard
Generated by RelativeCI Documentation Report issue
@@ -207,51 +190,6 @@ export const resizeGridStrategy: CanvasStrategyFactory = ( | |||
} | |||
} | |||
|
|||
type DimensionIndexes = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all this stuff now lives in grid-hepers.ts
type: 'REMOVE' | ||
} | ||
|
||
export type AlterGridTemplateDimensionAction = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really like the Action terminology, we use that for our dispatchable actions, someone would easily think these actions are EditorActions too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah good point
}, [] as ExpandedGridDimension[]) | ||
} | ||
|
||
function alterGridDimensions(params: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function does not generally alters the grid dimensions, but it alters the repeats, I would express this in the name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it does alter them, you can replace or remove any dimension, not just repeats.
**Problem:** As a followup to #6380 , we should preserve `repeat` functions & co. when altering the grid template from the inspector inputs too. **Fix:** 1. Factored out the logic to alter a table from the resize strategy to helper functions in `grid-helpers.ts` 2. Adjusted the logic to support removing elements too 3. Refactored the callbacks in the inspector section to use the new helpers Fixes #6387
Problem:
As a followup to #6380 , we should preserve
repeat
functions & co. when altering the grid template from the inspector inputs too.Fix:
grid-helpers.ts
Fixes #6387