feat(split): configure sizes per part via size and unit - #2323
Conversation
a2e6667 to
86c1253
Compare
There was a problem hiding this comment.
Code Review
This pull request refactors the split component by removing the sizes input from SiSplitComponent and introducing a required size input and an optional unit input ('px' or 'fr') directly on SiSplitPartComponent. This simplifies the API and improves layout behavior. The feedback highlights a potential TypeScript compilation error under strict type checking when restoring UI state, where state can be undefined, and suggests using a type guard to ensure type safety.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
bdb7b09 to
8cb85f4
Compare
2c1aa68 to
e8d6959
Compare
|
I think |
Actual, px and fr unit are the official abbreviations see https://developer.mozilla.org/en-US/docs/Web/CSS/Guides/Grid_layout/Basic_concepts#the_fr_unit |
Was not aware of this, then it's fine. Thanks for the clarification. |
spike-rabbit
left a comment
There was a problem hiding this comment.
After thinking again about, I really think we should have the unit as part of the size input.
I guess for compatibility we can keep a plain number size.
But the unit input, I am not seeing it.
Especially since it is rather easy to parse and handle internally.
It is actually also still type safe by using typescripts string template pattern.
There is no conflict with any existing events. Please check out my commit I pushed to your branch.
cf694a4 to
2a17f4f
Compare
6299f94 to
bfec8e0
Compare
BREAKING CHANGE: The relative sizes of split parts are now configured on each `si-split-part` via the required `size` and `unit` input (`'px' | 'fr'`), instead of the `sizes` array on `si-split`. The `sizes` input on `SiSplitComponent` has been removed and `SiSplitPartComponent.size` is now required. The `unit` input on `SiSplitPartComponent` is now required. - Remove `SiSplitComponent.sizes` input. - Add `SiSplitPartComponent.unit` input (`'px' | 'fr'`). - Make `SiSplitPartComponent.size` required. Migration: move the relative sizes from `[sizes]` on `si-split` onto the `size`/`unit` inputs of each `si-split-part`. Before: ```html <si-split [sizes]="[20, 60, 20]"> <si-split-part>Left</si-split-part> <si-split-part>Center</si-split-part> <si-split-part>Right</si-split-part> </si-split> ``` After: ```html <si-split> <si-split-part size="20" unit="fr">Left</si-split-part> <si-split-part size="60" unit="fr">Center</si-split-part> <si-split-part size="20" unit="fr">Right</si-split-part> </si-split> ``` Fixed pixel sizes are now expressed directly on the part: ```html <si-split> <si-split-part size="200" unit="px">Fixed 200px</si-split-part> <si-split-part size="1" unit="fr">Remaining space</si-split-part> </si-split> ```
f6ea37f to
26b17e0
Compare
BREAKING CHANGE: The relative sizes of split parts are now configured on
each
si-split-partvia the requiredsizeandunitinput (
'px' | 'fr'), instead of thesizesarray onsi-split.The
sizesinput onSiSplitComponenthas been removed andSiSplitPartComponent.sizeis now required.The
unitinput onSiSplitPartComponentis now required.SiSplitComponent.sizesinput.SiSplitPartComponent.unitinput ('px' | 'fr').SiSplitPartComponent.sizerequired.Migration: move the relative sizes from
[sizes]onsi-splitonto thesize/unitinputs of eachsi-split-part.Before:
After:
Fixed pixel sizes are now expressed directly on the part:
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: