Conversation
…n for resize controller
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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) | ||
| // ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
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 */ |
There was a problem hiding this comment.
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 | ||
| * * |
There was a problem hiding this comment.
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? |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
3 participants
No description provided.