feat: implement collapse/flip/rotate as buttons AND feat: add commons-demo iconset#160
feat: implement collapse/flip/rotate as buttons AND feat: add commons-demo iconset#160javier-godoy wants to merge 7 commits into
Conversation
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughAdds hide/rotate/flip source-viewer buttons backed by a new ChangesSource viewer controls and event-driven demo UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
base/src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.java (1)
375-384: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winToggle from the live layout orientation when the cache is unset.
If
source-rotatearrives beforeonAttach()finishes initializingsplitOrientation, this branch treatsnullas "not horizontal" and forcesHORIZONTAL, which is alreadySplitLayoutDemo's default. The first rotate click can therefore no-op. Derive the next value fromcurrentLayout.getOrientation()whensplitOrientationis stillnull.Proposed fix
private void toggleSplitterOrientation(boolean fromClient) { if (currentLayout == null) { return; } - if (Orientation.HORIZONTAL.equals(splitOrientation)) { - splitOrientation = Orientation.VERTICAL; - } else { - splitOrientation = Orientation.HORIZONTAL; - } - setOrientation(splitOrientation, fromClient); + Orientation currentOrientation = + splitOrientation != null ? splitOrientation : currentLayout.getOrientation(); + setOrientation( + Orientation.HORIZONTAL.equals(currentOrientation) + ? Orientation.VERTICAL + : Orientation.HORIZONTAL, + fromClient); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.java` around lines 375 - 384, The toggleSplitterOrientation method in TabbedDemo should not assume a null splitOrientation means non-horizontal; when the cache is unset, read the current value from currentLayout.getOrientation() and use that to compute the next orientation. Update the branch that flips between Orientation.HORIZONTAL and Orientation.VERTICAL so the first source-rotate event uses the live SplitLayout state instead of defaulting to HORIZONTAL, then pass the computed value into setOrientation.
🧹 Nitpick comments (7)
base/src/main/resources/META-INF/resources/frontend/styles/commons-demo/shared-styles.css (3)
135-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
scaleXinstead ofscalexper stylelint.Static analysis flags
scalexas a naming-convention violation (should bescaleX). CSS transform functions are case-insensitive so this works functionally, but fixing keeps it consistent with the linter config.✏️ Suggested fix
- &.source-code-viewer-rotate-button { transform: scalex(-1) rotate(90deg); } + &.source-code-viewer-rotate-button { transform: scaleX(-1) rotate(90deg); }and similarly for line 150.
Also applies to: 150-150
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/resources/META-INF/resources/frontend/styles/commons-demo/shared-styles.css` at line 135, Update the CSS transform functions in the shared-styles rules for source-code-viewer-rotate-button to use scaleX instead of scalex so they match the stylelint naming convention. Make the same correction in both affected declarations in the stylesheet, keeping the existing transform behavior unchanged.Source: Linters/SAST tools
98-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing blank line before declaration (stylelint).
Static analysis flags a missing empty line before the declaration at line 100.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/resources/META-INF/resources/frontend/styles/commons-demo/shared-styles.css` around lines 98 - 101, Add the missing blank line before the declaration block in the shared-styles.css rule for vaadin-button.source-code-viewer-button vaadin-icon to satisfy stylelint. Update that CSS selector block so the property declarations are separated by an empty line in the expected style format, keeping the fix localized to the vaadin-button.source-code-viewer-button vaadin-icon rule.Source: Linters/SAST tools
132-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComment whitespace nit flagged by stylelint.
/* Rotate the icons to match the layout disposition.*/is missing a space before*/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/resources/META-INF/resources/frontend/styles/commons-demo/shared-styles.css` at line 132, The comment in the shared stylesheet has a stylelint whitespace nit: the block comment closing delimiter is missing a space before */. Update the comment in the CSS file so the formatting matches the linter expectation, keeping the same wording but fixing the trailing spacing in the comment syntax.Source: Linters/SAST tools
base/src/main/java/com/flowingcode/vaadin/addons/demo/SourceCodeViewer.java (2)
116-163: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo detach cleanup for observers.
observeScrollbar()disconnects previous observers before creating new ones on reattach, but there's noaddDetachListenerto explicitly disconnect theResizeObserver/MutationObserverwhen the component is permanently removed. Consider disconnecting on detach for tidier resource lifecycle management, even though most browsers will eventually GC observers whose targets are detached.♻️ Suggested addition
addAttachListener(ev -> { fetchContents(url, language); observeScrollbar(); }); + addDetachListener(ev -> getElement().executeJs( + "this.__scrollbarObserver?.disconnect(); this.__scrollbarMutation?.disconnect();")); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/flowingcode/vaadin/addons/demo/SourceCodeViewer.java` around lines 116 - 163, The observeScrollbar() setup currently reconnects observers on reattach, but it does not explicitly clean them up when the component is detached. Update SourceCodeViewer to register a detach cleanup that disconnects root.__scrollbarObserver and root.__scrollbarMutation, and clears those references so the ResizeObserver and MutationObserver are disposed when the element is permanently removed.
134-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAncestor-walking loop is effectively a no-op.
Since
has-code-viewer-gutteris added torootitself (line 57), thewhileloop that walks upparentElementalways terminates immediately withtarget === root. The traversal logic never actually executes for the intended "find nearest ancestor" purpose in the current usage. If this was meant to support cases where the class is added to a wrapper other thanroot, that's fine as defensive code; otherwise it can be simplified to just userootdirectly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/flowingcode/vaadin/addons/demo/SourceCodeViewer.java` around lines 134 - 137, The ancestor search in SourceCodeViewer’s gutter handling is effectively redundant because has-code-viewer-gutter is already applied to root. Update the logic around the target/root lookup to either use root directly in the relevant handler or clearly constrain the parentElement walk to a real ancestor-finding use case; refer to SourceCodeViewer and the target loop near the gutter setup so the change stays localized.base/src/main/java/com/flowingcode/vaadin/addons/demo/CommonsDemoIcons.java (1)
67-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale Javadoc copied from another icon class.
The Javadoc says
Server side component for {@codeBrands}, which doesn't describe thisCommonsDemoIconsenum at all — looks like leftover text copied from another icon factory (e.g. a FontAwesome brands icon class).✏️ Suggested fix
/** - * Server side component for {`@code` Brands} + * Server side component for the {`@code` commons-demo} iconset. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/flowingcode/vaadin/addons/demo/CommonsDemoIcons.java` around lines 67 - 70, The Javadoc on CommonsDemoIcons is stale and references Brands, which does not match this enum. Update the comment above `@JsModule`("./commons-demo-iconset.ts") to accurately describe CommonsDemoIcons as the server-side icon set/component for the demo commons icons, and remove any copied wording from the other icon class.base/pom.xml (1)
320-321: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNon-descriptive profile id.
dancegives no indication this profile cleans frontend/Node build artifacts. Consider a descriptive id likeclean-frontendfor maintainability.♻️ Suggested rename
- <profile> - <id>dance</id> + <profile> + <id>clean-frontend</id>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/pom.xml` around lines 320 - 321, The Maven profile id `dance` is too vague in the profile definition and should be renamed in the `profile` block to something descriptive like `clean-frontend` so its purpose is obvious. Update the `id` inside the relevant profile in `pom.xml` and make sure any references to that profile name elsewhere in the build are changed to match the new identifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@base/pom.xml`:
- Around line 325-327: The maven-clean-plugin declaration in the base pom is
missing an explicit version and is not covered by shared pluginManagement, so
the build can resolve it unpredictably. Update the plugin entry in the pom where
maven-clean-plugin is declared to include a pinned version, or add that version
centrally under the repository’s pluginManagement so all modules use the same
reproducible plugin version.
In `@base/src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.java`:
- Around line 95-97: The source collapse state in TabbedDemo is being
overwritten with the viewport-driven mobile value, which makes the persisted
preference stick incorrectly. Update the addSourceCollapseListener handler and
the sourceCollapsed usage in the update logic so mobile auto-collapse only
affects the current layout state, while sourceCollapsed continues to represent
the user’s explicit collapse preference. Keep the mobile/default behavior
separate from the saved collapse flag, and ensure updateSplitterPosition still
reacts correctly without writing mobile-driven state back into sourceCollapsed.
In `@base/src/main/resources/META-INF/resources/frontend/commons-demo-iconset.ts`:
- Around line 91-124: The iconset is registered with the wrong intrinsic canvas
size, so update the Iconset.register call in commons-demo-iconset.ts to use the
24×24 canvas expected by the SVGs. Keep the existing template and icon
definitions, and adjust the commons-demo registration so vaadin-icon picks up
the correct default sizing without requiring per-usage overrides.
---
Outside diff comments:
In `@base/src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.java`:
- Around line 375-384: The toggleSplitterOrientation method in TabbedDemo should
not assume a null splitOrientation means non-horizontal; when the cache is
unset, read the current value from currentLayout.getOrientation() and use that
to compute the next orientation. Update the branch that flips between
Orientation.HORIZONTAL and Orientation.VERTICAL so the first source-rotate event
uses the live SplitLayout state instead of defaulting to HORIZONTAL, then pass
the computed value into setOrientation.
---
Nitpick comments:
In `@base/pom.xml`:
- Around line 320-321: The Maven profile id `dance` is too vague in the profile
definition and should be renamed in the `profile` block to something descriptive
like `clean-frontend` so its purpose is obvious. Update the `id` inside the
relevant profile in `pom.xml` and make sure any references to that profile name
elsewhere in the build are changed to match the new identifier.
In `@base/src/main/java/com/flowingcode/vaadin/addons/demo/CommonsDemoIcons.java`:
- Around line 67-70: The Javadoc on CommonsDemoIcons is stale and references
Brands, which does not match this enum. Update the comment above
`@JsModule`("./commons-demo-iconset.ts") to accurately describe CommonsDemoIcons
as the server-side icon set/component for the demo commons icons, and remove any
copied wording from the other icon class.
In `@base/src/main/java/com/flowingcode/vaadin/addons/demo/SourceCodeViewer.java`:
- Around line 116-163: The observeScrollbar() setup currently reconnects
observers on reattach, but it does not explicitly clean them up when the
component is detached. Update SourceCodeViewer to register a detach cleanup that
disconnects root.__scrollbarObserver and root.__scrollbarMutation, and clears
those references so the ResizeObserver and MutationObserver are disposed when
the element is permanently removed.
- Around line 134-137: The ancestor search in SourceCodeViewer’s gutter handling
is effectively redundant because has-code-viewer-gutter is already applied to
root. Update the logic around the target/root lookup to either use root directly
in the relevant handler or clearly constrain the parentElement walk to a real
ancestor-finding use case; refer to SourceCodeViewer and the target loop near
the gutter setup so the change stays localized.
In
`@base/src/main/resources/META-INF/resources/frontend/styles/commons-demo/shared-styles.css`:
- Line 135: Update the CSS transform functions in the shared-styles rules for
source-code-viewer-rotate-button to use scaleX instead of scalex so they match
the stylelint naming convention. Make the same correction in both affected
declarations in the stylesheet, keeping the existing transform behavior
unchanged.
- Around line 98-101: Add the missing blank line before the declaration block in
the shared-styles.css rule for vaadin-button.source-code-viewer-button
vaadin-icon to satisfy stylelint. Update that CSS selector block so the property
declarations are separated by an empty line in the expected style format,
keeping the fix localized to the vaadin-button.source-code-viewer-button
vaadin-icon rule.
- Line 132: The comment in the shared stylesheet has a stylelint whitespace nit:
the block comment closing delimiter is missing a space before */. Update the
comment in the CSS file so the formatting matches the linter expectation,
keeping the same wording but fixing the trailing spacing in the comment syntax.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9f7d9d6-cf1a-417e-a7f3-00eafb5fac6b
📒 Files selected for processing (10)
base/pom.xmlbase/src/main/java/com/flowingcode/vaadin/addons/demo/CommonsDemoIcons.javabase/src/main/java/com/flowingcode/vaadin/addons/demo/SourceCodeViewer.javabase/src/main/java/com/flowingcode/vaadin/addons/demo/SplitLayoutDemo.javabase/src/main/java/com/flowingcode/vaadin/addons/demo/TabbedDemo.javabase/src/main/java/com/flowingcode/vaadin/addons/demo/events/OrientationChangedEvent.javabase/src/main/java/com/flowingcode/vaadin/addons/demo/events/SourceCollapseChangedEvent.javabase/src/main/java/com/flowingcode/vaadin/addons/demo/events/SourcePositionChangedEvent.javabase/src/main/resources/META-INF/resources/frontend/commons-demo-iconset.tsbase/src/main/resources/META-INF/resources/frontend/styles/commons-demo/shared-styles.css
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-clean-plugin</artifactId> | ||
| <configuration> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether maven-clean-plugin version is managed in a parent POM
fd pom.xml | xargs rg -n 'maven-clean-plugin' -A3Repository: FlowingCode/CommonsDemo
Length of output: 402
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- base/pom.xml excerpt ---'
sed -n '300,350p' base/pom.xml
echo
echo '--- repository pom files ---'
fd -a pom.xml
echo
echo '--- maven-clean-plugin occurrences ---'
rg -n 'maven-clean-plugin|<pluginManagement>|<parent>|<version>' -g 'pom.xml' -A4 -B4 .Repository: FlowingCode/CommonsDemo
Length of output: 17541
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,45p' base/pom.xmlRepository: FlowingCode/CommonsDemo
Length of output: 1709
Pin maven-clean-plugin or manage it centrally base/pom.xml:324-327 — this plugin has no local <version>, and it isn’t defined in the repo’s pluginManagement, so the build depends on Maven’s default resolution. Pin a version here or add it to shared pluginManagement for reproducible builds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@base/pom.xml` around lines 325 - 327, The maven-clean-plugin declaration in
the base pom is missing an explicit version and is not covered by shared
pluginManagement, so the build can resolve it unpredictably. Update the plugin
entry in the pom where maven-clean-plugin is declared to include a pinned
version, or add that version centrally under the repository’s pluginManagement
so all modules use the same reproducible plugin version.
| const template = document.createElement('template'); | ||
| template.innerHTML = ` | ||
| <svg xmlns:svg="http://www.w3.org/2000/svg"> | ||
| <defs> | ||
| <g id="rotate"> | ||
| <!-- lucide rotate-cw-square --> | ||
| <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="lucide lucide-rotate-cw-square-icon lucide-rotate-cw-square"><path d="M12 5H6a2 2 0 0 0-2 2v3"/><path d="m9 8 3-3-3-3"/><path d="M4 14v4a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V7a2 2 0 0 0-2-2h-2"/></svg> | ||
| </g> | ||
| <g id="flip"> | ||
| <!-- lucide move-horizontal --> | ||
| <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="m18 8 4 4-4 4"/><path d="M2 12h20"/><path d="m6 8-4 4 4 4"/></svg> | ||
| </g> | ||
| <g id="show-source"> | ||
| <!-- lucide panel-left-close --> | ||
| <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect width="18" height="18" x="3" y="3" rx="2"/><path d="M9 3v18"/><path d="m16 15-3-3 3-3"/></svg> | ||
| </g> | ||
| <g id="hide-source"> | ||
| <!-- lucide panel-left-open--> | ||
| <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect width="18" height="18" x="3" y="3" rx="2"/><path d="M9 3v18"/><path d="m14 9 3 3-3 3"/></svg> | ||
| </g> | ||
| <g id="home"> | ||
| <!-- fas:home --> | ||
| <svg viewBox="0 0 576 512"><path d="M575.8 255.5c0 18-15 32.1-32 32.1l-32 0 .7 160.2c0 2.7-.2 5.4-.5 8.1l0 16.2c0 22.1-17.9 40-40 40l-16 0c-1.1 0-2.2 0-3.3-.1c-1.4 .1-2.8 .1-4.2 .1L416 512l-24 0c-22.1 0-40-17.9-40-40l0-24 0-64c0-17.7-14.3-32-32-32l-64 0c-17.7 0-32 14.3-32 32l0 64 0 24c0 22.1-17.9 40-40 40l-24 0-31.9 0c-1.5 0-3-.1-4.5-.2c-1.2 .1-2.4 .2-3.6 .2l-16 0c-22.1 0-40-17.9-40-40l0-112c0-.9 0-1.9 .1-2.8l0-69.7-32 0c-18 0-32-14-32-32.1c0-9 3-17 10-24L266.4 8c7-7 15-8 22-8s15 2 21 7L564.8 231.5c8 7 12 15 11 24z"/></svg> | ||
| </g> | ||
| <g id="github"> | ||
| <!-- fab:github --> | ||
| <svg viewBox="0 0 512 512"><path xmlns="http://www.w3.org/2000/svg" fill="currentColor" d="M173.9 397.4c0 2-2.3 3.6-5.2 3.6-3.3 .3-5.6-1.3-5.6-3.6 0-2 2.3-3.6 5.2-3.6 3-.3 5.6 1.3 5.6 3.6zm-31.1-4.5c-.7 2 1.3 4.3 4.3 4.9 2.6 1 5.6 0 6.2-2s-1.3-4.3-4.3-5.2c-2.6-.7-5.5 .3-6.2 2.3zm44.2-1.7c-2.9 .7-4.9 2.6-4.6 4.9 .3 2 2.9 3.3 5.9 2.6 2.9-.7 4.9-2.6 4.6-4.6-.3-1.9-3-3.2-5.9-2.9zM252.8 8c-138.7 0-244.8 105.3-244.8 244 0 110.9 69.8 205.8 169.5 239.2 12.8 2.3 17.3-5.6 17.3-12.1 0-6.2-.3-40.4-.3-61.4 0 0-70 15-84.7-29.8 0 0-11.4-29.1-27.8-36.6 0 0-22.9-15.7 1.6-15.4 0 0 24.9 2 38.6 25.8 21.9 38.6 58.6 27.5 72.9 20.9 2.3-16 8.8-27.1 16-33.7-55.9-6.2-112.3-14.3-112.3-110.5 0-27.5 7.6-41.3 23.6-58.9-2.6-6.5-11.1-33.3 2.6-67.9 20.9-6.5 69 27 69 27 20-5.6 41.5-8.5 62.8-8.5s42.8 2.9 62.8 8.5c0 0 48.1-33.6 69-27 13.7 34.7 5.2 61.4 2.6 67.9 16 17.7 25.8 31.5 25.8 58.9 0 96.5-58.9 104.2-114.8 110.5 9.2 7.9 17 22.9 17 46.4 0 33.7-.3 75.4-.3 83.6 0 6.5 4.6 14.4 17.3 12.1 100-33.2 167.8-128.1 167.8-239 0-138.7-112.5-244-251.2-244zM105.2 352.9c-1.3 1-1 3.3 .7 5.2 1.6 1.6 3.9 2.3 5.2 1 1.3-1 1-3.3-.7-5.2-1.6-1.6-3.9-2.3-5.2-1zm-10.8-8.1c-.7 1.3 .3 2.9 2.3 3.9 1.6 1 3.6 .7 4.3-.7 .7-1.3-.3-2.9-2.3-3.9-2-.6-3.6-.3-4.3 .7zm32.4 35.6c-1.6 1.3-1 4.3 1.3 6.2 2.3 2.3 5.2 2.6 6.5 1 1.3-1.3 .7-4.3-1.3-6.2-2.2-2.3-5.2-2.6-6.5-1zm-11.4-14.7c-1.6 1-1.6 3.6 0 5.9s4.3 3.3 5.6 2.3c1.6-1.3 1.6-3.9 0-6.2-1.4-2.3-4-3.3-5.6-2z"/></svg> | ||
| </g> | ||
| </defs> | ||
| </svg>`; | ||
|
|
||
| customElements.whenDefined('vaadin-iconset').then(Iconset=>{ | ||
| Iconset.register('commons-demo', 4, template); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What does the size parameter of Vaadin's Iconset.register(name, size, template) control?
💡 Result:
In Vaadin, the size parameter of the Iconset.register(name, size, template) method defines the intrinsic size of the icons within the iconset [1]. Because icons in Vaadin must be square, this value determines the width and height used for the SVG viewBox attribute [1][2][3]. When an icon is used in a component (such as vaadin-icon), the size defined in the iconset takes precedence over any size defined by the user to ensure the icon renders correctly [1].
Citations:
- 1: https://cdn.vaadin.com/vaadin-web-components/25.0.7/elements/vaadin-iconset/
- 2: https://cdn.vaadin.com/vaadin-web-components/25.0.7/elements/vaadin-icon/
- 3: https://origin.vaadin.com/docs/latest/components/icons
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the iconset file and nearby registrations/usages.
git ls-files 'base/src/main/resources/META-INF/resources/frontend/*iconset*' 'base/src/main/resources/META-INF/resources/frontend/*.ts' | sed -n '1,200p'
printf '\n--- commons-demo-iconset.ts ---\n'
sed -n '1,220p' base/src/main/resources/META-INF/resources/frontend/commons-demo-iconset.ts
printf '\n--- register usages ---\n'
rg -n "Iconset\.register\(" base/src/main/resources/META-INF/resources/frontend -g '*.ts'
printf '\n--- iconset-related usages ---\n'
rg -n "vaadin-iconset|register\('.*',\s*[0-9]+,\s*template\)" base/src/main/resources/META-INF/resources/frontend -g '*.ts'Repository: FlowingCode/CommonsDemo
Length of output: 8186
🌐 Web query:
Vaadin Iconset.register size parameter intrinsic icon canvas size viewBox default sizing
💡 Result:
In Vaadin, the Iconset.register method is used to register an icon collection, where the size parameter specifies the intrinsic square dimensions of the icons within that set [1][2]. This size value is fundamental because it directly determines the viewBox attribute of the SVG icons when they are rendered [3][4]. Key aspects of icon sizing and viewBox in Vaadin include: Intrinsic Size and viewBox: When you register an iconset, the size parameter (e.g., 16 or 24) defines the square coordinate system for the viewBox (e.g., 0 0 16 16) [1][3]. Icons must be square to ensure proper rendering [1][2]. Precedence: When using a vaadin-icon component, the size defined in the vaadin-iconset takes precedence over any size defined by the user on the individual icon instance to ensure a consistent appearance [1][2]. CSS Styling vs. Intrinsic Size: While the size parameter establishes the internal viewBox, the visual size of the icon on the screen is controlled by CSS [3][4]. The vaadin-icon component supports the --vaadin-icon-size custom CSS property to set the width and height of the icon container [3][4]. Defaults and Padding: Vaadin’s built-in icon sets have specific conventions. For example, Vaadin Icons are designed on a 16×16 pixel canvas but are typically rendered at 24×24 pixels [5]. Lumo Icons are defined on a 24×24 pixel canvas, which includes an internal 16×16 active area and 4 pixels of surrounding whitespace [5][6]. The visual size can be adjusted using CSS to manage internal padding when icons lack sufficient native whitespace [6]. When manually providing an SVG to a vaadin-icon (without an iconset), you can explicitly set the size property on the component, which will then update the viewBox accordingly [3][4].
Citations:
- 1: https://cdn.vaadin.com/vaadin-web-components/25.0.0/elements/vaadin-iconset/
- 2: https://cdn.vaadin.com/vaadin-web-components/25.0.7/elements/vaadin-iconset/
- 3: https://cdn.vaadin.com/vaadin-web-components/25.0.0/elements/vaadin-icon/
- 4: https://cdn.vaadin.com/vaadin-web-components/25.1.1/elements/vaadin-icon/
- 5: https://vaadin.com/docs/latest/components/icons/default-icons
- 6: https://origin.vaadin.com/docs/latest/components/icons
Use 24 for the iconset canvas size. Iconset.register('commons-demo', 4, template) sets the iconset’s intrinsic viewBox to 4×4, but these icons are authored on 24×24 canvases. That will produce the wrong default sizing in vaadin-icon unless every usage overrides it.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 91-119: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: template.innerHTML = <svg xmlns:svg="http://www.w3.org/2000/svg"> <defs> <g id="rotate"> <!-- lucide rotate-cw-square --> <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="lucide lucide-rotate-cw-square-icon lucide-rotate-cw-square"><path d="M12 5H6a2 2 0 0 0-2 2v3"/><path d="m9 8 3-3-3-3"/><path d="M4 14v4a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V7a2 2 0 0 0-2-2h-2"/></svg> </g> <g id="flip"> <!-- lucide move-horizontal --> <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="m18 8 4 4-4 4"/><path d="M2 12h20"/><path d="m6 8-4 4 4 4"/></svg> </g> <g id="show-source"> <!-- lucide panel-left-close --> <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect width="18" height="18" x="3" y="3" rx="2"/><path d="M9 3v18"/><path d="m16 15-3-3 3-3"/></svg> </g> <g id="hide-source"> <!-- lucide panel-left-open--> <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect width="18" height="18" x="3" y="3" rx="2"/><path d="M9 3v18"/><path d="m14 9 3 3-3 3"/></svg> </g> <g id="home"> <!-- fas:home --> <svg viewBox="0 0 576 512"><path d="M575.8 255.5c0 18-15 32.1-32 32.1l-32 0 .7 160.2c0 2.7-.2 5.4-.5 8.1l0 16.2c0 22.1-17.9 40-40 40l-16 0c-1.1 0-2.2 0-3.3-.1c-1.4 .1-2.8 .1-4.2 .1L416 512l-24 0c-22.1 0-40-17.9-40-40l0-24 0-64c0-17.7-14.3-32-32-32l-64 0c-17.7 0-32 14.3-32 32l0 64 0 24c0 22.1-17.9 40-40 40l-24 0-31.9 0c-1.5 0-3-.1-4.5-.2c-1.2 .1-2.4 .2-3.6 .2l-16 0c-22.1 0-40-17.9-40-40l0-112c0-.9 0-1.9 .1-2.8l0-69.7-32 0c-18 0-32-14-32-32.1c0-9 3-17 10-24L266.4 8c7-7 15-8 22-8s15 2 21 7L564.8 231.5c8 7 12 15 11 24z"/></svg> </g> <g id="github"> <!-- fab:github --> <svg viewBox="0 0 512 512"><path xmlns="http://www.w3.org/2000/svg" fill="currentColor" d="M173.9 397.4c0 2-2.3 3.6-5.2 3.6-3.3 .3-5.6-1.3-5.6-3.6 0-2 2.3-3.6 5.2-3.6 3-.3 5.6 1.3 5.6 3.6zm-31.1-4.5c-.7 2 1.3 4.3 4.3 4.9 2.6 1 5.6 0 6.2-2s-1.3-4.3-4.3-5.2c-2.6-.7-5.5 .3-6.2 2.3zm44.2-1.7c-2.9 .7-4.9 2.6-4.6 4.9 .3 2 2.9 3.3 5.9 2.6 2.9-.7 4.9-2.6 4.6-4.6-.3-1.9-3-3.2-5.9-2.9zM252.8 8c-138.7 0-244.8 105.3-244.8 244 0 110.9 69.8 205.8 169.5 239.2 12.8 2.3 17.3-5.6 17.3-12.1 0-6.2-.3-40.4-.3-61.4 0 0-70 15-84.7-29.8 0 0-11.4-29.1-27.8-36.6 0 0-22.9-15.7 1.6-15.4 0 0 24.9 2 38.6 25.8 21.9 38.6 58.6 27.5 72.9 20.9 2.3-16 8.8-27.1 16-33.7-55.9-6.2-112.3-14.3-112.3-110.5 0-27.5 7.6-41.3 23.6-58.9-2.6-6.5-11.1-33.3 2.6-67.9 20.9-6.5 69 27 69 27 20-5.6 41.5-8.5 62.8-8.5s42.8 2.9 62.8 8.5c0 0 48.1-33.6 69-27 13.7 34.7 5.2 61.4 2.6 67.9 16 17.7 25.8 31.5 25.8 58.9 0 96.5-58.9 104.2-114.8 110.5 9.2 7.9 17 22.9 17 46.4 0 33.7-.3 75.4-.3 83.6 0 6.5 4.6 14.4 17.3 12.1 100-33.2 167.8-128.1 167.8-239 0-138.7-112.5-244-251.2-244zM105.2 352.9c-1.3 1-1 3.3 .7 5.2 1.6 1.6 3.9 2.3 5.2 1 1.3-1 1-3.3-.7-5.2-1.6-1.6-3.9-2.3-5.2-1zm-10.8-8.1c-.7 1.3 .3 2.9 2.3 3.9 1.6 1 3.6 .7 4.3-.7 .7-1.3-.3-2.9-2.3-3.9-2-.6-3.6-.3-4.3 .7zm32.4 35.6c-1.6 1.3-1 4.3 1.3 6.2 2.3 2.3 5.2 2.6 6.5 1 1.3-1.3 .7-4.3-1.3-6.2-2.2-2.3-5.2-2.6-6.5-1zm-11.4-14.7c-1.6 1-1.6 3.6 0 5.9s4.3 3.3 5.6 2.3c1.6-1.3 1.6-3.9 0-6.2-1.4-2.3-4-3.3-5.6-2z"/></svg> </g> </defs> </svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation
(dom-content-modification)
[warning] 91-119: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: template.innerHTML = <svg xmlns:svg="http://www.w3.org/2000/svg"> <defs> <g id="rotate"> <!-- lucide rotate-cw-square --> <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="lucide lucide-rotate-cw-square-icon lucide-rotate-cw-square"><path d="M12 5H6a2 2 0 0 0-2 2v3"/><path d="m9 8 3-3-3-3"/><path d="M4 14v4a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V7a2 2 0 0 0-2-2h-2"/></svg> </g> <g id="flip"> <!-- lucide move-horizontal --> <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="m18 8 4 4-4 4"/><path d="M2 12h20"/><path d="m6 8-4 4 4 4"/></svg> </g> <g id="show-source"> <!-- lucide panel-left-close --> <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect width="18" height="18" x="3" y="3" rx="2"/><path d="M9 3v18"/><path d="m16 15-3-3 3-3"/></svg> </g> <g id="hide-source"> <!-- lucide panel-left-open--> <svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><rect width="18" height="18" x="3" y="3" rx="2"/><path d="M9 3v18"/><path d="m14 9 3 3-3 3"/></svg> </g> <g id="home"> <!-- fas:home --> <svg viewBox="0 0 576 512"><path d="M575.8 255.5c0 18-15 32.1-32 32.1l-32 0 .7 160.2c0 2.7-.2 5.4-.5 8.1l0 16.2c0 22.1-17.9 40-40 40l-16 0c-1.1 0-2.2 0-3.3-.1c-1.4 .1-2.8 .1-4.2 .1L416 512l-24 0c-22.1 0-40-17.9-40-40l0-24 0-64c0-17.7-14.3-32-32-32l-64 0c-17.7 0-32 14.3-32 32l0 64 0 24c0 22.1-17.9 40-40 40l-24 0-31.9 0c-1.5 0-3-.1-4.5-.2c-1.2 .1-2.4 .2-3.6 .2l-16 0c-22.1 0-40-17.9-40-40l0-112c0-.9 0-1.9 .1-2.8l0-69.7-32 0c-18 0-32-14-32-32.1c0-9 3-17 10-24L266.4 8c7-7 15-8 22-8s15 2 21 7L564.8 231.5c8 7 12 15 11 24z"/></svg> </g> <g id="github"> <!-- fab:github --> <svg viewBox="0 0 512 512"><path xmlns="http://www.w3.org/2000/svg" fill="currentColor" d="M173.9 397.4c0 2-2.3 3.6-5.2 3.6-3.3 .3-5.6-1.3-5.6-3.6 0-2 2.3-3.6 5.2-3.6 3-.3 5.6 1.3 5.6 3.6zm-31.1-4.5c-.7 2 1.3 4.3 4.3 4.9 2.6 1 5.6 0 6.2-2s-1.3-4.3-4.3-5.2c-2.6-.7-5.5 .3-6.2 2.3zm44.2-1.7c-2.9 .7-4.9 2.6-4.6 4.9 .3 2 2.9 3.3 5.9 2.6 2.9-.7 4.9-2.6 4.6-4.6-.3-1.9-3-3.2-5.9-2.9zM252.8 8c-138.7 0-244.8 105.3-244.8 244 0 110.9 69.8 205.8 169.5 239.2 12.8 2.3 17.3-5.6 17.3-12.1 0-6.2-.3-40.4-.3-61.4 0 0-70 15-84.7-29.8 0 0-11.4-29.1-27.8-36.6 0 0-22.9-15.7 1.6-15.4 0 0 24.9 2 38.6 25.8 21.9 38.6 58.6 27.5 72.9 20.9 2.3-16 8.8-27.1 16-33.7-55.9-6.2-112.3-14.3-112.3-110.5 0-27.5 7.6-41.3 23.6-58.9-2.6-6.5-11.1-33.3 2.6-67.9 20.9-6.5 69 27 69 27 20-5.6 41.5-8.5 62.8-8.5s42.8 2.9 62.8 8.5c0 0 48.1-33.6 69-27 13.7 34.7 5.2 61.4 2.6 67.9 16 17.7 25.8 31.5 25.8 58.9 0 96.5-58.9 104.2-114.8 110.5 9.2 7.9 17 22.9 17 46.4 0 33.7-.3 75.4-.3 83.6 0 6.5 4.6 14.4 17.3 12.1 100-33.2 167.8-128.1 167.8-239 0-138.7-112.5-244-251.2-244zM105.2 352.9c-1.3 1-1 3.3 .7 5.2 1.6 1.6 3.9 2.3 5.2 1 1.3-1 1-3.3-.7-5.2-1.6-1.6-3.9-2.3-5.2-1zm-10.8-8.1c-.7 1.3 .3 2.9 2.3 3.9 1.6 1 3.6 .7 4.3-.7 .7-1.3-.3-2.9-2.3-3.9-2-.6-3.6-.3-4.3 .7zm32.4 35.6c-1.6 1.3-1 4.3 1.3 6.2 2.3 2.3 5.2 2.6 6.5 1 1.3-1.3 .7-4.3-1.3-6.2-2.2-2.3-5.2-2.6-6.5-1zm-11.4-14.7c-1.6 1-1.6 3.6 0 5.9s4.3 3.3 5.6 2.3c1.6-1.3 1.6-3.9 0-6.2-1.4-2.3-4-3.3-5.6-2z"/></svg> </g> </defs> </svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation
(unsafe-html-content-assignment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@base/src/main/resources/META-INF/resources/frontend/commons-demo-iconset.ts`
around lines 91 - 124, The iconset is registered with the wrong intrinsic canvas
size, so update the Iconset.register call in commons-demo-iconset.ts to use the
24×24 canvas expected by the SVGs. Keep the existing template and icon
definitions, and adjust the commons-demo registration so vaadin-icon picks up
the correct default sizing without requiring per-usage overrides.
|
|
Close in favor of #161 |
1 participant
Close #123
Summary by CodeRabbit