feat(joint-core/demo/container): render collapse button via highlighter #2972
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements improvements to container components by rendering collapse/expand buttons via a custom highlighter instead of embedding them directly in the markup. Key changes include refactoring container shape definitions, updating the toggle behavior and event handling in the diagram, and adding custom CSS for improved visual consistency.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/joint-core/demo/container/src/joint.shapes.container.js | Refactored container shapes definitions, updated constants, and modified the toggle method to delegate collapse/expand rendering. |
| packages/joint-core/demo/container/src/index.js | Introduced a cellNamespace, implemented an ExpandButtonHighlighter to handle button rendering, and revised event handlers for container size updates. |
| packages/joint-core/demo/container/css/container.css | Added CSS styles for the paper border and margin adjustments. |
Comments suppressed due to low confidence (1)
packages/joint-core/demo/container/src/joint.shapes.container.js:139
- The toggle method no longer updates the button icon attribute, so please ensure that the highlighter component reliably updates the collapse/expand icon to avoid any visual inconsistencies.
this.set('collapsed', typeof shouldCollapse === 'boolean' ? shouldCollapse : !this.get('collapsed'));
| if (!joint.shapes.container.Parent.isContainer(container)) return; | ||
| const flags = { ignoreCommandManager: true }; | ||
| if (container.isCollapsed()) { | ||
| container.resize(140, 30, flags); |
There was a problem hiding this comment.
The hard-coded dimensions (140, 30) for the collapsed state are used directly; consider extracting these values into named constants to improve clarity and ease future adjustments.
| container.resize(140, 30, flags); | |
| container.resize(COLLAPSED_WIDTH, COLLAPSED_HEIGHT, flags); |
Copilot uses AI. Check for mistakes.
| } else { | ||
| d = 'M -4 0 4 0'; | ||
| } | ||
| this.childNodes.icon.setAttribute('d', d); |
There was a problem hiding this comment.
Before directly accessing this.childNodes.icon, it would be safer to check that the icon element exists to prevent potential runtime errors if the rendering process fails.
| this.childNodes.icon.setAttribute('d', d); | |
| if (this.childNodes.icon) { | |
| this.childNodes.icon.setAttribute('d', d); | |
| } else { | |
| console.error('Icon element is missing. Unable to set attribute "d".'); | |
| } |
Copilot uses AI. Check for mistakes.
Description
Renders containers' collapse/expand buttons with a highlighter, instead of hard-coding them in their markup.
Motivation and Context
Using highlighters for this purpose is a better practice, since they can be mixed-and-matched with any shape.