-
Notifications
You must be signed in to change notification settings - Fork 10
Splitter component #1969
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
base: master
Are you sure you want to change the base?
Splitter component #1969
Conversation
…n for resize controller
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.
Pull request overview
The Splitter component provides a framework for creating resizable and collapsible split-pane layouts in both horizontal and vertical orientations.
Changes:
- Added a new Splitter web component with comprehensive resize and collapse functionality
- Implemented keyboard navigation and pointer-based interactions for pane resizing
- Added support for size constraints (min/max) and multiple size units (px, %, auto)
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| stories/splitter.stories.ts | Storybook stories demonstrating splitter component usage and configurations |
| stories/card.stories.ts | Enhanced component and property descriptions with more detailed explanations |
| src/index.ts | Export added for the new Splitter component |
| src/components/types.ts | Added SplitterOrientation type definition |
| src/components/splitter/themes/splitter.base.scss | Base styles for splitter component and its sub-parts |
| src/components/splitter/splitter.ts | Core implementation of the Splitter component with resize logic |
| src/components/splitter/splitter.spec.ts | Comprehensive test suite covering all splitter functionality |
| src/components/resize-container/types.ts | Added optional updateTarget configuration flag |
| src/components/resize-container/resize-controller.ts | Conditional position updates based on updateTarget flag |
| src/components/common/definitions/defineAllComponents.ts | Registration of Splitter component |
| src/components/common/context.ts | Added splitter context for component communication |
| [part='start-expand-btn'] { | ||
| background-color: red; | ||
| } | ||
|
|
||
| [part='end-collapse-btn'], | ||
| [part='end-expand-btn'] { | ||
| background-color: green; | ||
| } | ||
|
|
||
| [part='drag-handle'] { | ||
| background-color: yellow; |
Copilot
AI
Feb 11, 2026
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.
The debugging color values (red, green, yellow) should be removed before merging to production. These hardcoded background colors appear to be temporary styling for development purposes and should either be removed or replaced with proper theme variables.
| [part='start-expand-btn'] { | |
| background-color: red; | |
| } | |
| [part='end-collapse-btn'], | |
| [part='end-expand-btn'] { | |
| background-color: green; | |
| } | |
| [part='drag-handle'] { | |
| background-color: yellow; | |
| [part='start-expand-btn'], | |
| [part='end-collapse-btn'], | |
| [part='end-expand-btn'] { | |
| background-color: var(--ig-gray-400); | |
| } | |
| [part='drag-handle'] { | |
| background-color: var(--ig-gray-200); |
| document.addEventListener('DOMContentLoaded', () => { | ||
| // const splitter = document.getElementById( | ||
| // 'splitter' | ||
| // ) as IgcSplitterComponent; | ||
| // splitter.addEventListener('igcResizeStart', (event) => | ||
| // console.log(event.detail) | ||
| // ); | ||
| // splitter.addEventListener('igcResizing', (event) => | ||
| // console.log(event.detail) | ||
| // ); | ||
| // splitter.addEventListener('igcResizeEnd', (event) => | ||
| // console.log(event.detail) | ||
| // ); | ||
| }); | ||
|
|
Copilot
AI
Feb 11, 2026
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.
Remove or uncomment this commented-out event listener code. If the code is needed for testing, it should be uncommented; otherwise, it should be deleted to avoid confusion and maintain code cleanliness.
| document.addEventListener('DOMContentLoaded', () => { | |
| // const splitter = document.getElementById( | |
| // 'splitter' | |
| // ) as IgcSplitterComponent; | |
| // splitter.addEventListener('igcResizeStart', (event) => | |
| // console.log(event.detail) | |
| // ); | |
| // splitter.addEventListener('igcResizing', (event) => | |
| // console.log(event.detail) | |
| // ); | |
| // splitter.addEventListener('igcResizeEnd', (event) => | |
| // console.log(event.detail) | |
| // ); | |
| }); |
|
|
||
| .splitters { | ||
| height: 400px; | ||
| /*width: 1000px;*/ /* useful for testing % values */ |
Copilot
AI
Feb 11, 2026
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.
Remove or uncomment this commented-out width style. Commented code with inline notes suggesting it's 'useful for testing' should either be properly implemented as a story variant or removed entirely.
| /*width: 1000px;*/ /* useful for testing % values */ |
| * into multiple smaller resizable and collapsible areas. | ||
| * | ||
| * @element igc-splitter | ||
| * * |
Copilot
AI
Feb 11, 2026
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.
Remove the extra asterisk. There appears to be a duplicate asterisk in the JSDoc comment which should be removed.
| * * | |
| * |
| } | ||
| } | ||
|
|
||
| // TODO: should there be events on expand/collapse - existing resize events or others? |
Copilot
AI
Feb 11, 2026
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 TODO comment should be resolved before merging. The decision about whether expand/collapse operations should emit events is a design decision that should be finalized, as it affects the component's API.
| // TODO: should there be events on expand/collapse - existing resize events or others? | |
| // Note: expand/collapse actions do not emit dedicated events; consumers can rely on resize events to react to size changes. |
|
|
||
| /** Toggles the collapsed state of the pane. */ | ||
| public toggle(position: 'start' | 'end') { | ||
| // TODO: determine behavior when disableCollapsed is true |
Copilot
AI
Feb 11, 2026
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 TODO comment should be resolved before merging. The behavior when disableCollapse is true needs to be clearly defined and implemented.
| // TODO: determine behavior when disableCollapsed is true | |
| // When collapsing is disabled, programmatic toggling is also disabled. | |
| if ((this as unknown as { disableCollapse?: boolean }).disableCollapse) { | |
| return; | |
| } |
…teUI/igniteui-webcomponents into mkirkova/splitter-component
No description provided.