feat(buttons): add square btn-icon #1215
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new Sequence Diagram(s)Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (5)
projects/element-theme/src/styles/bootstrap/_button-group.scss(2 hunks)projects/element-theme/src/styles/bootstrap/_buttons.scss(2 hunks)src/app/examples/buttons/button-groups.html(1 hunks)src/app/examples/buttons/buttons.html(1 hunks)src/app/examples/buttons/split-button.html(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
src/app/examples/buttons/split-button.html (2)
10-10: LGTM: Consistent size modifier applied to dropdown toggles.The
btn-smclass has been consistently applied to all split-button dropdown toggles withbtn-icon. This aligns with the broader styling updates introducing standardized sizing for icon buttons.Also applies to: 28-28, 45-45, 60-60
74-74: LGTM: Icon-only buttons now use standardized classes.The standalone icon buttons now properly use
btn-icon btn-sm, ensuring visual consistency with their adjacent dropdown-toggle siblings.Also applies to: 91-91
src/app/examples/buttons/button-groups.html (1)
45-78: LGTM: Size modifiers applied consistently to icon button groups.All icon buttons across Primary, Secondary, and Tertiary groups now include the
btn-smmodifier. This ensures uniform sizing and aligns with the centralized.btn-iconstyling rules introduced in the SCSS updates.projects/element-theme/src/styles/bootstrap/_button-group.scss (1)
89-96: LGTM with note:!importantusage is now consistent.The
!importantdeclarations for.btn-iconand.icon-onlypadding are now consistent with the new.btn-iconblock at lines 55-60. This ensures uniform enforcement of padding across all contexts.Note: The same considerations about
!importantnecessity apply here as mentioned in the previous comment.projects/element-theme/src/styles/bootstrap/_buttons.scss (5)
69-71: LGTM: Icon buttons excluded from minimum width constraint.Adding
.btn-iconto the exclusion list ensures icon-only buttons size naturally to their content, consistent with.btn-circlebehavior.
151-160: LGTM: Icon and circle buttons consolidated.The expanded
:is()selector properly groups.btn-iconwith.btn-circleand.btn-close, ensuring consistent padding and icon margins for all icon-only button variants.
162-168: LGTM: Clear distinction between circular and square icon buttons.This is the core feature of the PR. The separate border-radius declarations properly differentiate:
.btn-circle: Fully circular buttons (50%).btn-icon: Square buttons with rounded corners (semantic token)This provides the UI flexibility to choose between circular and square icon button variants.
170-189: LGTM: Unified sizing for icon button variants.Both
.btn-circleand.btn-iconshare the same size calculations, which ensures visual consistency while allowing different border-radius styling. Good code reuse.
191-198: LGTM: Ghost variant extended to icon buttons.Adding
.btn-icon.btn-ghostensures the ghost button styling is available for square icon buttons, maintaining parity with circular variants.src/app/examples/buttons/buttons.html (2)
152-165: LGTM: Close button variants added with proper accessibility.The new close button examples include proper
aria-labelattributes and consistent disabled state bindings.
239-290: LGTM: Comprehensive small icon button examples.The small icon button examples mirror the large variants with consistent structure and accessibility attributes. The same aria-label considerations mentioned in the previous comment apply here (e.g., line 263, 270 use
aria-label="badge").
|
Documentation. Coverage Reports: |
|
@dauriamarco I think we should move the square buttons above the circle ones as they should be the new default icon buttons: 
|
dauriamarco
force-pushed
the
feat/btn-icon
branch
from
13b30d4 to
afdc294
Compare
dauriamarco
force-pushed
the
feat/btn-icon
branch
2 times, most recently
from
885cf6a to
592ca02
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/examples/buttons/buttons.html (1)
37-107: Good ordering! Square icon buttons placed before circle buttons.The new large icon button variants (
btn-icon btn-lg) are correctly positioned before the circle button variants, which aligns with kfenner's comment that square buttons should be the new default icon buttons and should appear first in the examples.Note: Generic aria-labels like "badge" have already been flagged in a previous review comment.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (5)
projects/element-theme/src/styles/bootstrap/_button-group.scssprojects/element-theme/src/styles/bootstrap/_buttons.scsssrc/app/examples/buttons/button-groups.htmlsrc/app/examples/buttons/buttons.htmlsrc/app/examples/buttons/split-button.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
src/app/examples/buttons/buttons.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
src/app/examples/buttons/split-button.html (1)
10-10: LGTM! Consistent btn-sm application across split buttons.The addition of
btn-smto all icon button variants in split button configurations is consistent and aligns well with the new.btn-iconstyling introduced in the SCSS changes.Also applies to: 28-28, 45-45, 60-60, 74-74, 79-79, 91-91, 96-96
src/app/examples/buttons/button-groups.html (1)
45-78: LGTM! Consistent icon button sizing in button groups.The
btn-smmodifier has been consistently applied to all icon button variants across Primary, Secondary, and Tertiary button groups, ensuring uniform sizing and alignment with the new.btn-iconstyling rules.projects/element-theme/src/styles/bootstrap/_button-group.scss (1)
55-58: LGTM! The!importantconcern has been addressed.The new
.btn-iconrule within.btn-groupcorrectly sets icon button sizing with calculated padding that accounts for border width. The previous review concern about!importantusage has been resolved—the current implementation relies on selector specificity instead.src/app/examples/buttons/buttons.html (2)
109-160: LGTM! Comprehensive small icon button variants.The small icon button variants (
btn-icon btn-sm) provide a complete set of examples across all button types and include close button variants, demonstrating the full range of the new square icon button API.
277-289: LGTM! Close button variants complete the circle button examples.The addition of close button variants (
btn-circle btn-close) within the small circle button group provides comprehensive examples of all available button combinations.projects/element-theme/src/styles/bootstrap/_buttons.scss (2)
162-168: Clear separation of border-radius logic.The explicit border-radius rules for
.btn-circle(50%) and.btn-icon(semantic token) provide clear, maintainable styling that distinguishes circular from square icon buttons.
170-198: LGTM! Unified sizing and visual states for icon buttons.The shared sizing implementation for both
.btn-circleand.btn-iconensures consistency across button variants, and the inclusion of.btn-icon.btn-ghostin the visual state mapping completes the integration of the new square icon button variant.
dauriamarco
force-pushed
the
feat/btn-icon
branch
from
592ca02 to
b49aa52
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/examples/buttons/buttons.html (1)
37-107: Icon button placement aligns with design direction, but improve aria-labels.The large icon button examples are well-positioned before the circle variants, aligning with kfenner's suggestion that square buttons should be the new default. However, multiple buttons use generic
aria-label="badge"(lines 62, 69, 76, 83, 90, 97), which doesn't demonstrate accessibility best practices for example/demo code.Consider more descriptive labels to better demonstrate proper accessibility patterns:
- aria-label="badge" + aria-label="View badge"Or vary the labels based on context (e.g., "Secondary badge", "Warning badge", "Danger badge") to demonstrate that each button should have a unique, descriptive label.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (5)
projects/element-theme/src/styles/bootstrap/_button-group.scssprojects/element-theme/src/styles/bootstrap/_buttons.scsssrc/app/examples/buttons/button-groups.htmlsrc/app/examples/buttons/buttons.htmlsrc/app/examples/buttons/split-button.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
src/app/examples/buttons/buttons.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (9)
projects/element-theme/src/styles/bootstrap/_buttons.scss (5)
69-71: LGTM: Icon buttons correctly excluded from min-inline-size constraint.The addition of
.btn-iconto the exclusion list is appropriate, ensuring icon buttons use their fixed dimensions rather than the minimum width constraint applied to standard buttons.
151-160: Excellent consolidation of shared button variant styles.The consolidated selector correctly groups
.btn-circle,.btn-icon, and.btn-closewith shared padding and icon margin rules, while properly excluding button-group contexts. This aligns with the previously approved refactoring approach.
162-168: Clean separation of border-radius rules for circle vs. icon buttons.The explicit border-radius rules effectively distinguish circular (50%) from square/rounded icon buttons (semantic token). The use of semantic tokens for
.btn-iconensures consistent theming.
170-189: Well-structured consolidation of circle and icon button sizing.Sharing the sizing rules for
.btn-circleand.btn-iconis appropriate since both variants use identical dimensions. The unified sizing across.btn-lg,.btn-sm, and.btn-xsmaintains consistency while adhering to DRY principles.
191-198: Ghost styling correctly extended to icon buttons.Adding
.btn-icon.btn-ghostto the existing ghost button styles ensures visual consistency across circle, icon, and close button variants.src/app/examples/buttons/button-groups.html (1)
43-79: Consistent application of btn-sm to icon button groups.All icon buttons within button groups now include the
btn-smmodifier, ensuring consistent sizing across Primary, Secondary, and Tertiary variants. This aligns well with the new.btn-iconstyling infrastructure.src/app/examples/buttons/split-button.html (1)
10-10: Consistent btn-sm application across split button icon variants.The addition of
btn-smto all icon buttons and dropdown toggles in split button configurations ensures uniform sizing and visual consistency throughout the component.Also applies to: 28-28, 45-45, 60-60, 74-74, 79-79, 91-91, 96-96
projects/element-theme/src/styles/bootstrap/_button-group.scss (1)
55-58: Clean button-group integration for icon buttons.The
.btn-iconrule properly scopes icon button styling within button groups, allowing natural sizing withinline-size: unsetand consistent padding calculation. The removal of!importantdeclarations (per previous feedback) improves maintainability.src/app/examples/buttons/buttons.html (1)
277-290: Circle close button examples complete the variant showcase.The addition of close button examples (with and without
btn-tertiary) within the circle button group provides comprehensive coverage of button variants. Thearia-label="close"is appropriately descriptive for these controls.
dauriamarco
force-pushed
the
feat/btn-icon
branch
3 times, most recently
from
71734a5 to
ef66a90
Compare
kfenner
left a comment
kfenner
left a comment
There was a problem hiding this comment.
@dauriamarco Looks great, thank you 🙏
I'd like to adjust the commit message slightly:
feat(buttons): add square icon buttons
Introduces square icon buttons (`.btn-icon`) in addition to the existing circle
buttons (`.btn-circle`). They follow the same sizing options (lg, sm, xs) and
integrate seamlessly with button groups.
NOTE: The new square icon buttons (`.btn-icon`) are introduced as the
new default style for icon buttons. The existing circle buttons (`.btn-circle`)
are now reserved for cases where a circular shape is explicitly required.
ef66a90 to
980e89a
Compare
kfenner
left a comment
kfenner
left a comment
There was a problem hiding this comment.
We cannot merge this until we have an alignment of sizing for buttons and icon buttons.
@panch1739 I understood that you would like to postpone this but we cannot ship this as-is for v49. I think you should have a discussion with iX about this. Would be good to understand the differences in button sizing between Element and iX.
Also, @hbxes pointed out, that our (non-icon) button in small size is 28px which is not defined in the design system. So maybe we should also do something about that.
dauriamarco
force-pushed
the
feat/btn-icon
branch
from
980e89a to
294f5c1
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @projects/element-ng/datepicker/si-calendar-button.component.ts:
- Line 51: In the template for SiCalendarButtonComponent remove the conflicting
"btn-xs" token from the class list so the element only uses the intended
"btn-lg" sizing; locate the class attribute string containing "btn btn-circle
btn-lg btn-tertiary btn-xs position-absolute end-0 top-0 me-2 mt-2" and delete
"btn-xs" so it reads without that token.
In @projects/element-ng/datepicker/si-date-range.component.html:
- Line 33: Button contains conflicting size classes ("btn-lg" and "btn-xs");
remove the inappropriate size modifier so only the intended size remains. In the
template element that currently has class="btn btn-circle btn-lg btn-tertiary
btn-xs" (the calendar button in si-date-range component), delete "btn-xs" and
leave "btn-lg" (or replace with the project's agreed size class) so there are no
mutually-exclusive size classes applied.
In @projects/element-ng/filter-bar/si-filter-pill.component.html:
- Line 23: The button element in si-filter-pill.component.html has conflicting
size classes "btn-lg" and "btn-xs"; remove the incorrect modifier so only one
size remains—prefer keeping "btn-lg" (remove "btn-xs") to match other
remove/close buttons, updating the class attribute on the element with
class="btn-remove element-cancel btn btn-circle btn-lg btn-ghost focus-inside
ms-0".
In @projects/element-ng/status-bar/si-status-bar.component.html:
- Line 110: Update the mute button's class attribute to use the new icon-button
pattern: replace "btn-circle" with "btn-icon" on the element that currently has
class="bar-item mute-button btn btn-circle btn-tertiary ms-5"; if you need to
keep the circular shape retain both by using class="bar-item mute-button btn
btn-icon btn-circle btn-tertiary ms-5", otherwise use the new square default
class="bar-item mute-button btn btn-icon btn-tertiary ms-5".
In @projects/element-ng/tour/si-tour.component.html:
- Line 32: The close button class string "btn btn-circle btn-ghost mt-4 me-5"
was changed to remove btn-sm which enlarges the button and may break the tour
header flex layout; verify header alignment and spacing and either reintroduce
btn-sm (restore "btn-sm" in the same element) or adjust surrounding
spacing/margins so the title/progress area and the button remain visually
balanced, and optionally migrate the button from the circular variant to the new
square variant by replacing "btn-circle" with "btn-icon" to match the new design
language (apply these changes on the same button element in
si-tour.component.html and confirm visually across breakpoints).
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (10)
playwright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-no-backdrop-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-no-backdrop-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--scroll-mode-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--scroll-mode-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel-collapsible--open-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel-collapsible--open-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (53)
docs/components/layout-navigation/modals.mdplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons.yamlprojects/dashboards-demo/src/app/components/widget-catalog/custom-widget-catalog.component.htmlprojects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.htmlprojects/element-ng/action-modal/si-alert-dialog/si-alert-dialog.component.htmlprojects/element-ng/action-modal/si-confirmation-dialog/si-confirmation-dialog.component.htmlprojects/element-ng/action-modal/si-delete-confirmation-dialog/si-delete-confirmation-dialog.component.htmlprojects/element-ng/action-modal/si-edit-discard-dialog/si-edit-discard-dialog.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.htmlprojects/element-ng/column-selection-dialog/si-column-selection-dialog.component.htmlprojects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/datepicker/si-calendar-button.component.tsprojects/element-ng/datepicker/si-date-range.component.htmlprojects/element-ng/file-uploader/si-file-uploader.component.htmlprojects/element-ng/filter-bar/si-filter-pill.component.htmlprojects/element-ng/main-detail-container/si-main-detail-container.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/notification-item/si-notification-item.component.htmlprojects/element-ng/photo-upload/si-photo-upload.component.htmlprojects/element-ng/side-panel/si-side-panel-content.component.htmlprojects/element-ng/slider/si-slider.component.htmlprojects/element-ng/status-bar/si-status-bar.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.htmlprojects/element-ng/tour/si-tour.component.htmlprojects/element-ng/tree-view/si-tree-view.component.htmlprojects/element-ng/wizard/si-wizard.component.htmlprojects/element-theme/src/styles/bootstrap/_button-group.scssprojects/element-theme/src/styles/bootstrap/_buttons.scsssrc/app/examples/buttons/buttoncategories.htmlsrc/app/examples/buttons/buttons.htmlsrc/app/examples/datatable/bootstrap.htmlsrc/app/examples/datatable/datatable-grouped-rows.htmlsrc/app/examples/si-chat-messages/si-attachment-list.htmlsrc/app/examples/si-chat-messages/si-chat-container.htmlsrc/app/examples/si-chat-messages/si-chat-input.htmlsrc/app/examples/si-chat-messages/si-chat-message.htmlsrc/app/examples/si-dashboard/si-list-widget-css.htmlsrc/app/examples/si-datepicker/si-modal-service.htmlsrc/app/examples/si-list-details/si-list-details-router.htmlsrc/app/examples/si-list-details/si-list-details.htmlsrc/app/examples/si-map/si-map-custom-cluster-popover.tssrc/app/examples/si-modals/app-table.component.htmlsrc/app/examples/si-modals/si-modal-service.htmlsrc/app/examples/si-notification-item/si-notification-item.htmlsrc/app/examples/si-wizard/si-wizard-forms.html
🧰 Additional context used
🧠 Learnings (16)
📚 Learning: 2025-12-22T13:04:35.578Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:35.578Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
projects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/filter-bar/si-filter-pill.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.htmlprojects/element-ng/notification-item/si-notification-item.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/tour/si-tour.component.htmlprojects/element-ng/file-uploader/si-file-uploader.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/main-detail-container/si-main-detail-container.component.htmlprojects/element-ng/tree-view/si-tree-view.component.htmlprojects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.htmlprojects/element-ng/chat-messages/si-user-message.component.html
📚 Learning: 2025-12-09T14:31:33.543Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:14-50
Timestamp: 2025-12-09T14:31:33.543Z
Learning: In projects/element-ng/chat-messages/si-ai-chat-container.component.html, message IDs can change, so tracking by message.id is not appropriate. The component uses `track message` (object identity) which is the correct approach for this use case.
Applied to files:
src/app/examples/si-chat-messages/si-chat-message.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.htmlsrc/app/examples/si-chat-messages/si-chat-container.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-user-message.component.html
📚 Learning: 2025-12-09T14:32:34.036Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:21-47
Timestamp: 2025-12-09T14:32:34.036Z
Learning: In projects/element-ng/chat-messages, nested if blocks are preferred over switch statements for branching on message types in Angular templates.
Applied to files:
src/app/examples/si-chat-messages/si-chat-message.htmlprojects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.html
📚 Learning: 2025-12-09T14:32:26.166Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:21-47
Timestamp: 2025-12-09T14:32:26.166Z
Learning: In Angular templates, prefer using nested *ngIf blocks over *ngSwitch for branching on message types within the chat-messages components. Use *ngIf with else branches to handle distinct cases and avoid complex switch logic in HTML templates. This guideline applies to all HTML templates under projects/element-ng/chat-messages.
Applied to files:
projects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-user-message.component.html
📚 Learning: 2025-11-18T12:37:30.510Z
Learnt from: Killusions
Repo: siemens/element PR: 1040
File: projects/element-ng/chat-messages/si-chat-input.component.ts:338-349
Timestamp: 2025-11-18T12:37:30.510Z
Learning: In projects/element-ng/chat-messages/si-chat-input.component.ts, the interrupt behavior is intentionally different for button clicks vs Enter key: the button can emit interrupt even without content, but pressing Enter requires content or attachments (canSend() must be true) to emit interrupt.
Applied to files:
projects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-user-message.component.html
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
projects/element-ng/datepicker/si-calendar-button.component.tssrc/app/examples/si-map/si-map-custom-cluster-popover.tsprojects/element-ng/datepicker/components/si-calendar-direction-button.component.ts
📚 Learning: 2025-12-30T13:52:33.581Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:33.581Z
Learning: Maintain the existing negative naming convention (e.g., disableBackdrop) across the siemens/element repository for consistency. Do not refactor to positive names (e.g., showBackdrop, hasBackdrop) unless the project explicitly changes the convention. In reviews, flag changes that alter established naming patterns in related files under projects/element-ng.
Applied to files:
projects/element-ng/datepicker/si-calendar-button.component.tsprojects/element-ng/datepicker/components/si-calendar-direction-button.component.ts
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/side-panel/si-side-panel-content.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/threshold/si-threshold.component.html
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/tour/si-tour.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlsrc/app/examples/buttons/buttons.html
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-map/si-map-custom-cluster-popover.ts
📚 Learning: 2025-12-22T13:29:36.922Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/color-scheme.part.ts:35-35
Timestamp: 2025-12-22T13:29:36.922Z
Learning: In the Element design system (projects/element-ng/theme and projects/element-theme), only `--element-body-font-family` is exposed as a CSS custom property for typography. Font sizes are NOT exposed as CSS custom properties; they are defined using internal SCSS variables (e.g., `typography.$si-font-size-body`) that compile to fixed values. Using hardcoded pixel values for fontSize in AG Grid theme configuration (e.g., `fontSize: '14px'`) is the correct approach.
Applied to files:
projects/element-theme/src/styles/bootstrap/_buttons.scss
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
src/app/examples/buttons/buttons.html
📚 Learning: 2026-01-02T16:41:57.274Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:57.274Z
Learning: In Element documentation (siemens/element), sizing utility examples in docs may intentionally differ from implementation examples to demonstrate multiple valid approaches - for example, documentation might use traditional CSS properties like `height` and `width` while the implementation example uses logical properties like `block-size` and `inline-size`. These are considered different examples rather than inconsistencies.
Applied to files:
src/app/examples/buttons/buttons.html
📚 Learning: 2026-01-02T16:41:48.359Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:48.359Z
Learning: In Element docs, sizing examples may differ between documentation and implementation to illustrate multiple valid approaches. Do not flag such differences as inconsistencies; treat them as separate illustrative examples (e.g., using height/width in docs vs block-size/inline-size in implementation). Apply this understanding across Markdown documentation files to avoid false-positive review flags.
Applied to files:
docs/components/layout-navigation/modals.md
📚 Learning: 2025-12-01T09:34:50.469Z
Learnt from: Killusions
Repo: siemens/element PR: 1047
File: docs/fundamentals/ux-text-style-guide/basics.md:30-30
Timestamp: 2025-12-01T09:34:50.469Z
Learning: In docs/fundamentals/ux-text-style-guide/basics.md, the h4 heading level (####) for "Do's" and "Don'ts" within the dos-and-donts structure is intentional, even though it triggers the MD001 linting rule about heading hierarchy.
Applied to files:
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons.yaml
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (60)
projects/element-ng/slider/si-slider.component.html (1)
4-4: Consistent with button-sizing standardization.The removal of
btn-smfrom both increment and decrement buttons aligns with the PR's broader effort to standardize icon/circle button sizing. The buttons remain appropriately classified asbtn-circle btn-secondary.Please verify that the larger button size (default instead of small) displays appropriately within the slider layout and doesn't cause any spacing or alignment issues. Additionally, confirm the visual appearance on touch/mobile viewports, where larger targets may be beneficial for usability.
Also applies to: 62-62
projects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.html (2)
34-34: Verify button variant choice aligns with PR objectives.The PR introduces square buttons as the new default for icon buttons, and the PR comment from kfenner suggests square buttons should replace circular ones as the default. However, this close button retains
btn-circle. Please confirm whether this is intentional (e.g., toast notifications specifically retain circular buttons) or if it should be updated to use the square variant for consistency with the new design direction.
34-34: Verify visual impact of removingbtn-smwith existing negative margins.Removing
btn-smincreases the button size from small to default, but the negative margins (mt-n2 me-n2) remain unchanged. These margins were likely calibrated for the smaller button size. Please verify that the larger button doesn't cause layout issues or visual misalignment in the toast notification, and adjust margins if needed.src/app/examples/si-notification-item/si-notification-item.html (1)
71-71: LGTM! Verify visual appearance in notification items.The removal of
btn-smfrom these circular buttons aligns with the PR's standardization effort. The changes are consistent across all circular buttons in this template, and accessibility attributes are properly maintained.Since notification items can be space-constrained, verify that the larger default-sized buttons don't negatively impact layout, spacing, or visual density in the notification UI.
Also applies to: 78-78, 113-113
projects/element-ng/datepicker/components/si-calendar-direction-button.component.ts (1)
24-24: LGTM! Verify the button shape choice and visual appearance.The removal of
btn-smcorrectly aligns with the PR's broader button sizing standardization. However, based on the PR comments suggesting that square buttons should be the new default for icon buttons, please confirm:
- Is the use of
btn-circleintentional for calendar direction buttons, or should these be updated to the squarebtn-iconvariant introduced in this PR?- Does the larger default size work well visually within the calendar component layout?
- Are snapshot tests updated to reflect this size change (if applicable)?
projects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.html (1)
31-31: This file was newly added as part of commit 294f5c1 ("refactor(buttons): align button sizing across text and icon variants"), which unifies button sizing across the codebase. The current code is correct:btn-circleis the intended class for icon-only buttons on lines 31, 54, 69, and 81, and there is nobtn-smto remove since this is a new component. The BREAKING CHANGE in that commit addresses button sizing (e.g., btn-sm now represents 28px height instead of 32px), not a replacement ofbtn-circlewith a new square button variant. The original review assumptions about btn-sm removal and square button replacement do not apply to this component.Likely an incorrect or invalid review comment.
projects/element-ng/tree-view/si-tree-view.component.html (2)
11-18: LGTM - consistent with expand all button.The
btn-smremoval is consistent with the expand all button above and aligns with the broader refactoring effort.
3-10: No changes needed—.btn-circleis correct and consistent.The
btn-smremoval is appropriate and aligns with the refactoring objectives. These expand/collapse buttons correctly use.btn-circlefor circular styling, not.btn-icon(which is for square buttons). This is consistent with all other expand/collapse buttons throughout the codebase (status-bar, side-panel, navbar-vertical, etc.).projects/element-ng/action-modal/si-edit-discard-dialog/si-edit-discard-dialog.component.html (1)
9-9: LGTM! Consistent button sizing standardization.The removal of
btn-smfrom the modal close button aligns with the PR's objective to standardize icon button sizing. This presentational change makes the cancel button use the default size, consistent with the new squarebtn-iconvariant being introduced as the default.src/app/examples/si-modals/si-modal-service.html (1)
32-32: LGTM! Consistent sizing across all modal close buttons.All three modal close buttons have been updated consistently by removing
btn-sm, standardizing them to the default size. This aligns with the PR's systematic approach to button sizing normalization across modal templates.Also applies to: 52-52, 78-78
src/app/examples/si-chat-messages/si-attachment-list.html (1)
20-20: LGTM! Consistent with modal sizing standardization.The removal of
btn-smfrom the attachment modal's close button maintains consistency with the broader button sizing standardization across the codebase.projects/element-ng/action-modal/si-delete-confirmation-dialog/si-delete-confirmation-dialog.component.html (1)
9-9: LGTM! Component sizing aligned with new standards.The removal of
btn-smfrom the delete confirmation dialog's close button aligns this component with the standardized button sizing approach being implemented across the library.src/app/examples/datatable/datatable-grouped-rows.html (1)
21-21: LGTM! Sizing standardization extends to datatable controls.The removal of
btn-smfrom the group expand/collapse button demonstrates the consistent application of the new button sizing standard beyond just modal close buttons, maintaining visual coherence across different UI contexts.projects/element-ng/action-modal/si-alert-dialog/si-alert-dialog.component.html (1)
9-9: LGTM - Consistent sizing standardization.The removal of
btn-smaligns with the PR's goal to standardize button sizing across components. The change is purely presentational and preserves all functionality.projects/element-ng/wizard/si-wizard.component.html (1)
124-124: LGTM - Consistent sizing standardization.The removal of
btn-smfrom the horizontal cancel button is consistent with the standardization effort across the codebase.projects/element-ng/photo-upload/si-photo-upload.component.html (1)
64-64: LGTM - Consistent sizing standardization.The removal of
btn-smfrom the modal header cancel button is consistent with the standardization effort.projects/element-ng/column-selection-dialog/si-column-selection-dialog.component.html (1)
5-5: LGTM - Consistent sizing standardization.The removal of
btn-smfrom the cancel button completes the consistent sizing standardization across modal dialogs.src/app/examples/si-datepicker/si-modal-service.html (1)
9-9: LGTM!Removing
btn-smfrom the modal close button aligns with the PR's goal of standardizing button sizes across the component library.src/app/examples/si-list-details/si-list-details.html (1)
23-23: LGTM!Removing
btn-smfrom both the options and filters icon buttons standardizes their sizing with the rest of the component library.Also applies to: 41-41
src/app/examples/si-wizard/si-wizard-forms.html (1)
8-8: LGTM!Removing
btn-smfrom the modal close button is consistent with the standardization effort across modal components.projects/element-ng/chat-messages/si-user-message.component.html (1)
19-19: LGTM!Removing
btn-smfrom both action buttons standardizes their sizing. The changes maintain consistent styling for icon buttons within chat messages.Also applies to: 31-31
projects/element-ng/chat-messages/si-attachment-list.component.html (1)
47-47: LGTM! Button size standardization improves accessibility.Removing
btn-smincreases the remove button to default size, providing a larger touch target which improves usability and accessibility. This change aligns with the PR-wide effort to standardize icon button sizing.projects/element-ng/action-modal/si-confirmation-dialog/si-confirmation-dialog.component.html (1)
9-9: LGTM! Consistent modal close button sizing.Removing
btn-smstandardizes the close button size across all modal dialogs, improving consistency and click target size.projects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.html (1)
26-26: LGTM! Action button sizing improved.Removing
btn-smprovides better touch target for the list widget action button while maintaining all functionality and custom padding.projects/dashboards-demo/src/app/components/widget-catalog/custom-widget-catalog.component.html (1)
5-5: LGTM! Demo app aligned with library button sizing.Removing
btn-smensures the demo application's modal close button matches the standardized sizing across the core library components.src/app/examples/si-chat-messages/si-chat-container.html (2)
31-31: LGTM! Chat action button sizing standardized.Removing
btn-smfrom the chat message action button improves touch target size and aligns with the button sizing standardization.
80-80: LGTM! Modal close button sizing consistent.Removing
btn-smensures the preview modal's close button matches the standardized sizing across all modal dialogs.src/app/examples/si-map/si-map-custom-cluster-popover.ts (1)
26-26: LGTM! Button sizing standardization applied correctly.The removal of
btn-smaligns with the PR's goal of standardizing button sizes across the component library. The button retains appropriate styling withbtn-circleandbtn-ghost.projects/element-ng/threshold/si-threshold.component.html (1)
8-8: LGTM! Consistent button sizing updates.The removal of
btn-smfrom both the delete button (line 8) and add button (line 53) standardizes the threshold component's button sizing, aligning with the PR's broader refactoring goals.Also applies to: 53-53
docs/components/layout-navigation/modals.md (1)
82-82: LGTM! Documentation updated to reflect standardized button sizing.The removal of
btn-smfrom the modal cancel button example ensures the documentation aligns with the updated button sizing standards being implemented across the component library.src/app/examples/si-chat-messages/si-chat-message.html (1)
11-11: LGTM! Consistent button sizing updates across chat message actions.The removal of
btn-smfrom all three chat message action buttons standardizes their sizing and aligns with the PR's broader button refactoring objectives.Also applies to: 22-22, 43-43
projects/element-ng/main-detail-container/si-main-detail-container.component.html (1)
94-94: LGTM! Button sizing standardization applied correctly.The removal of
btn-smfrom the back button aligns with the PR's objective to standardize icon and circular button sizing across the component library.src/app/examples/si-list-details/si-list-details-router.html (1)
23-23: LGTM! Consistent button sizing standardization.Both circular buttons (options and filter) correctly remove
btn-smto align with the new default sizing for icon buttons throughout the library.Also applies to: 41-41
projects/element-ng/file-uploader/si-file-uploader.component.html (1)
64-64: LGTM! File uploader button sizing standardized.All three circular action buttons (retry, remove, cancel) correctly remove
btn-smwhile preserving their functionality, accessibility attributes, and event handlers.Also applies to: 76-76, 87-87
projects/element-ng/chat-messages/si-ai-message.component.html (1)
16-16: LGTM! AI message action buttons standardized.Both action buttons correctly remove
btn-smto adopt the standardized default sizing for circular icon buttons, while maintaining all functionality and accessibility.Also applies to: 28-28
projects/element-ng/chat-messages/si-chat-input.component.html (1)
53-53: LGTM! Chat input buttons fully standardized.All four circular buttons (attachment, actions, secondary actions, and main action button) correctly remove
btn-smto align with the standardized icon button sizing, while preserving all functionality, event handlers, and accessibility attributes.Also applies to: 67-67, 79-79, 100-100
projects/element-ng/notification-item/si-notification-item.component.html (4)
112-112: LGTM: Button sizing standardization.The removal of
btn-smaligns with the PR's goal to standardize icon button sizing to default size.
122-122: LGTM: Consistent sizing update.The sizing change is consistent with the overall standardization effort in this PR.
140-140: LGTM: Button sizing aligned.Consistent removal of
btn-smfor size standardization.
152-152: LGTM: Menu button sizing standardized.The sizing update completes the consistent standardization across all notification item action buttons.
src/app/examples/buttons/buttons.html (3)
47-127: LGTM: Icon buttons positioned correctly per PR requirements.The square icon buttons section is correctly positioned above the circle buttons section, aligning with the feedback that square buttons should be the new default icon buttons.
223-273: Verify that icon button sizing addresses the confusion mentioned in issue #1222.The Icon button sizing section demonstrates all size variants (lg, default, sm, xs) for both square and circle icon buttons. However, past comments indicate sizing confusion that needs resolution for v49. Ensure this implementation aligns with the resolution plan for issue #1222.
47-127: Improved aria-labels address previous feedback.The aria-labels have been enhanced from generic "badge" to include button variant context (e.g., "secondary badge", "tertiary warning badge"). This is an improvement over the previous iteration noted in past reviews.
Also applies to: 129-214
projects/element-ng/navbar-vertical/si-navbar-vertical.component.html (1)
20-20: LGTM: Sizing standardization for navbar toggle.The removal of
btn-smstandardizes the collapse toggle button to default size, consistent with the PR's sizing changes across the codebase.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (1)
91-91: LGTM: Button sizing standardized (note: summary inconsistency).The button sizing update is consistent with the broader PR changes. However, the AI summary incorrectly states "removed btn-ghost" when the code clearly shows
btn-ghostis still present. Based on the pattern across other files,btn-smwas likely removed.src/app/examples/si-modals/app-table.component.html (1)
6-6: LGTM: Modal close button sizing standardized.The removal of
btn-smaligns with the consistent sizing updates for close buttons across modal components in this PR.src/app/examples/si-chat-messages/si-chat-input.html (1)
27-27: LGTM - Button sizing standardizationThe removal of
btn-smaligns with the PR's button sizing standardization effort, changing the emoji picker button from small to default size while preserving all functionality and attributes.src/app/examples/si-dashboard/si-list-widget-css.html (1)
20-20: LGTM - Consistent sizing across list actionsThe removal of
btn-smfrom all six list item action buttons consistently standardizes their size to default, maintaining uniform appearance across the widget.Also applies to: 31-31, 42-42, 53-53, 64-64, 75-75
src/app/examples/buttons/buttoncategories.html (3)
12-19: LGTM - Circular navigation buttons now prominent with btn-lgAdding
btn-lgto the left/right navigation buttons increases their size appropriately, making them more prominent in the button category examples.
24-36: LGTM - Standardized sizing for icon buttonsRemoving
btn-smfrom the plus, delete, and options buttons standardizes them to default size, which aligns with the broader sizing refactor.
1-39: Missing square btn-icon demonstration per PR objectivesAccording to the PR title ("feat(buttons): add square btn-icon") and kfenner's comment, square buttons using the new
btn-iconclass should be demonstrated in this buttons example page, positioned above the circle buttons as the new default icon buttons. However, nobtn-iconbuttons are visible in this template.Should this file include examples of the new square
btn-iconvariant? Based on the PR objectives, it appears square icon buttons should be added here to showcase the new feature.projects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.html (1)
29-35: LGTM - Standardized sizing for editor controlsRemoving
btn-smfrom both the visibility toggle and drag-handle buttons standardizes their size to default while preserving all functionality, state management, and accessibility attributes.Also applies to: 39-47
projects/element-ng/side-panel/si-side-panel-content.component.html (1)
17-35: LGTM - Comprehensive sizing standardization across side panel controlsRemoving
btn-smfrom all five side panel header buttons (navigate links, fullscreen, close, and collapse toggle) consistently standardizes their size to default. All functionality, conditional rendering, accessibility attributes, and event bindings remain intact.Also applies to: 38-50, 56-66, 70-76, 80-89
src/app/examples/datatable/bootstrap.html (1)
41-41: LGTM! Consistent button size standardization.The removal of
btn-smfrom these circular icon buttons standardizes them to the default size, aligning with the broader refactoring effort across the project to consolidate icon button sizing.Also applies to: 75-75, 104-104, 133-133
projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.html (1)
10-12: LGTM! Icon button sizing standardized.The removal of
btn-smfrom the circular edit button aligns with the project-wide button sizing standardization. The single-line ternary format also improves readability.projects/element-theme/src/styles/bootstrap/_button-group.scss (1)
55-58: LGTM! Proper button group integration for icon buttons.The new
.btn-iconrule correctly integrates icon buttons into the button group sizing system by resetting the width constraint and applying consistent padding. The calculation accounts for border width using spacer tokens, matching the established pattern in this file (e.g., line 89).projects/element-theme/src/styles/bootstrap/_buttons.scss (4)
11-18: Well-structured icon button size variables.The new
$btn-icon-*variables provide clear, consistent sizing tokens for icon buttons across all size variants (default, lg, sm, xs). The naming convention aligns with existing button sizing patterns.
154-171: Excellent consolidation enabling composability.The refactored structure successfully achieves multiple goals:
Consolidated shared styles (lines 155-163): The
:is()selector unifies display, padding, and icon margin handling for all icon-related button variants while correctly excluding button-group contexts.Composable architecture (lines 165-171): Separating border-radius rules allows developers to combine classes (
btn btn-icon btn-circle) to create circular icon buttons, with.btn-iconas the base and.btn-circleas a modifier.This design addresses the feedback from past reviews about making square the default with circle as a variation.
173-195: LGTM! Unified sizing across icon button variants.The shared sizing block correctly applies consistent dimensions and font sizes to both
.btn-circleand.btn-iconacross all size variants (default, lg, sm, xs), eliminating duplication while maintaining flexibility.
197-204: Good addition of ghost styling for icon buttons.Adding
.btn-icon.btn-ghost(line 198) ensures the ghost variant works correctly with the new square icon buttons, maintaining styling consistency with the existing circular ghost buttons.
dauriamarco
force-pushed
the
feat/btn-icon
branch
from
294f5c1 to
46d7a09
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/app/examples/si-dashboard/si-list-widget-css.html:
- Line 20: The refactor removed btn-sm from some button instances but left other
identical patterns (small variants) in btn-icon and btn-circle buttons; search
for remaining "btn-sm" usages in the UI examples—specifically in
src/app/examples/buttons/buttons.html around the btn-icon and btn-circle
examples (lines noted in review) and either remove the btn-sm class to match the
refactor or explicitly document/annotate those occurrences as intentional
small-variant examples; update the markup for the matching examples (btn-icon,
btn-circle) to remove btn-sm or add a comment clarifying they are deliberate
examples so the change is consistent across the codebase.
In @src/app/examples/si-modals/si-modal-service.html:
- Line 32: One close button elsewhere still has a size modifier; locate the
close button element using the classes "btn-circle" and "element-cancel" (the
one that also includes "btn-xs") and remove the "btn-xs" class so it matches the
other modal header close buttons (i.e., only "btn-circle btn-ghost
element-cancel" remain); ensure no other size modifiers are present on similar
"btn-circle element-cancel" buttons to keep sizes consistent.
In @src/app/examples/si-wizard/si-wizard-forms.html:
- Line 8: Replace the circular icon button class with the new square variant:
locate the close button element that currently has classes "btn btn-circle
btn-ghost element-cancel" (use the element with class element-cancel to find it)
and change the styling class from btn-circle to btn-icon so it becomes "btn
btn-icon btn-ghost element-cancel", leaving other classes intact.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (48)
docs/components/layout-navigation/modals.mdprojects/dashboards-demo/src/app/components/widget-catalog/custom-widget-catalog.component.htmlprojects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.htmlprojects/element-ng/action-modal/si-alert-dialog/si-alert-dialog.component.htmlprojects/element-ng/action-modal/si-confirmation-dialog/si-confirmation-dialog.component.htmlprojects/element-ng/action-modal/si-delete-confirmation-dialog/si-delete-confirmation-dialog.component.htmlprojects/element-ng/action-modal/si-edit-discard-dialog/si-edit-discard-dialog.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.htmlprojects/element-ng/column-selection-dialog/si-column-selection-dialog.component.htmlprojects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/file-uploader/si-file-uploader.component.htmlprojects/element-ng/main-detail-container/si-main-detail-container.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/notification-item/si-notification-item.component.htmlprojects/element-ng/photo-upload/si-photo-upload.component.htmlprojects/element-ng/side-panel/si-side-panel-content.component.htmlprojects/element-ng/slider/si-slider.component.htmlprojects/element-ng/status-bar/si-status-bar.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.htmlprojects/element-ng/tour/si-tour.component.htmlprojects/element-ng/tree-view/si-tree-view.component.htmlprojects/element-ng/wizard/si-wizard.component.htmlprojects/element-theme/src/styles/bootstrap/_buttons.scsssrc/app/examples/buttons/buttoncategories.htmlsrc/app/examples/buttons/buttons.htmlsrc/app/examples/datatable/bootstrap.htmlsrc/app/examples/datatable/datatable-grouped-rows.htmlsrc/app/examples/si-chat-messages/si-attachment-list.htmlsrc/app/examples/si-chat-messages/si-chat-container.htmlsrc/app/examples/si-chat-messages/si-chat-input.htmlsrc/app/examples/si-chat-messages/si-chat-message.htmlsrc/app/examples/si-dashboard/si-list-widget-css.htmlsrc/app/examples/si-datepicker/si-modal-service.htmlsrc/app/examples/si-list-details/si-list-details-router.htmlsrc/app/examples/si-list-details/si-list-details.htmlsrc/app/examples/si-map/si-map-custom-cluster-popover.tssrc/app/examples/si-modals/app-table.component.htmlsrc/app/examples/si-modals/si-modal-service.htmlsrc/app/examples/si-notification-item/si-notification-item.htmlsrc/app/examples/si-wizard/si-wizard-forms.html
🧰 Additional context used
🧠 Learnings (14)
📚 Learning: 2025-12-09T14:31:33.543Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:14-50
Timestamp: 2025-12-09T14:31:33.543Z
Learning: In projects/element-ng/chat-messages/si-ai-chat-container.component.html, message IDs can change, so tracking by message.id is not appropriate. The component uses `track message` (object identity) which is the correct approach for this use case.
Applied to files:
src/app/examples/si-chat-messages/si-chat-container.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlsrc/app/examples/si-chat-messages/si-chat-message.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/side-panel/si-side-panel-content.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.html
📚 Learning: 2025-12-22T13:04:35.578Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:35.578Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
projects/element-ng/tour/si-tour.component.htmlprojects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.htmlprojects/element-ng/status-bar/si-status-bar.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.htmlprojects/element-ng/file-uploader/si-file-uploader.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/tree-view/si-tree-view.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/tour/si-tour.component.htmlsrc/app/examples/buttons/buttons.htmlprojects/element-ng/status-bar/si-status-bar.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.html
📚 Learning: 2026-01-02T16:41:48.359Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:48.359Z
Learning: In Element docs, sizing examples may differ between documentation and implementation to illustrate multiple valid approaches. Do not flag such differences as inconsistencies; treat them as separate illustrative examples (e.g., using height/width in docs vs block-size/inline-size in implementation). Apply this understanding across Markdown documentation files to avoid false-positive review flags.
Applied to files:
docs/components/layout-navigation/modals.md
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
src/app/examples/buttons/buttons.html
📚 Learning: 2026-01-02T16:41:57.274Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:57.274Z
Learning: In Element documentation (siemens/element), sizing utility examples in docs may intentionally differ from implementation examples to demonstrate multiple valid approaches - for example, documentation might use traditional CSS properties like `height` and `width` while the implementation example uses logical properties like `block-size` and `inline-size`. These are considered different examples rather than inconsistencies.
Applied to files:
src/app/examples/buttons/buttons.html
📚 Learning: 2025-12-22T13:16:19.243Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/icon/element-icons.ts:166-167
Timestamp: 2025-12-22T13:16:19.243Z
Learning: In projects/element-ng/icon/element-icons.ts, the elementCheckedImageShape icon intentionally uses base64 encoding instead of utf8 encoding (which is used for other icons). This approach is considered safer and correct for this specific icon and should not be flagged as an inconsistency.
Applied to files:
projects/element-ng/status-bar/si-status-bar.component.html
📚 Learning: 2025-12-09T14:32:26.166Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:21-47
Timestamp: 2025-12-09T14:32:26.166Z
Learning: In Angular templates, prefer using nested *ngIf blocks over *ngSwitch for branching on message types within the chat-messages components. Use *ngIf with else branches to handle distinct cases and avoid complex switch logic in HTML templates. This guideline applies to all HTML templates under projects/element-ng/chat-messages.
Applied to files:
projects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2025-11-18T12:37:30.510Z
Learnt from: Killusions
Repo: siemens/element PR: 1040
File: projects/element-ng/chat-messages/si-chat-input.component.ts:338-349
Timestamp: 2025-11-18T12:37:30.510Z
Learning: In projects/element-ng/chat-messages/si-chat-input.component.ts, the interrupt behavior is intentionally different for button clicks vs Enter key: the button can emit interrupt even without content, but pressing Enter requires content or attachments (canSend() must be true) to emit interrupt.
Applied to files:
projects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-map/si-map-custom-cluster-popover.ts
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
src/app/examples/si-map/si-map-custom-cluster-popover.tsprojects/element-ng/datepicker/components/si-calendar-direction-button.component.ts
📚 Learning: 2025-12-22T13:29:36.922Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/color-scheme.part.ts:35-35
Timestamp: 2025-12-22T13:29:36.922Z
Learning: In the Element design system (projects/element-ng/theme and projects/element-theme), only `--element-body-font-family` is exposed as a CSS custom property for typography. Font sizes are NOT exposed as CSS custom properties; they are defined using internal SCSS variables (e.g., `typography.$si-font-size-body`) that compile to fixed values. Using hardcoded pixel values for fontSize in AG Grid theme configuration (e.g., `fontSize: '14px'`) is the correct approach.
Applied to files:
projects/element-theme/src/styles/bootstrap/_buttons.scss
📚 Learning: 2025-12-30T13:52:33.581Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:33.581Z
Learning: Maintain the existing negative naming convention (e.g., disableBackdrop) across the siemens/element repository for consistency. Do not refactor to positive names (e.g., showBackdrop, hasBackdrop) unless the project explicitly changes the convention. In reviews, flag changes that alter established naming patterns in related files under projects/element-ng.
Applied to files:
projects/element-ng/datepicker/components/si-calendar-direction-button.component.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (64)
projects/element-ng/wizard/si-wizard.component.html (1)
124-124: LGTM! Consistent sizing with other wizard buttons.Removing
btn-smaligns this cancel button with the other circle buttons in the horizontal wizard (lines 194, 211), which already use the default size. This change is consistent with the PR's objective to establish a unified default size for icon and circle buttons.src/app/examples/si-map/si-map-custom-cluster-popover.ts (1)
24-31: LGTM! Button sizing change aligns with PR objectives.The removal of
btn-smis consistent with the PR's goal to establish a new default size for icon buttons across the codebase. The button functionality, accessibility (aria-label), and event handling remain unchanged—this is purely a visual styling update.projects/element-ng/action-modal/si-confirmation-dialog/si-confirmation-dialog.component.html (1)
7-14: LGTM! Button size refactoring aligns with PR objectives.The removal of
btn-smfrom the cancel button is consistent with the PR's goal to establish a new default size for icon buttons. The button maintains proper accessibility with thearia-labelattribute and correct event handling.projects/element-ng/tour/si-tour.component.html (1)
32-32: LGTM - sizing concern addressed.Thank you for clarifying that the button sizing system has been realigned across the board. The removal of
btn-smmaintains the intended size while simplifying the button styling system, which aligns with the PR's objectives.projects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.html (1)
29-29: LGTM! Consistent with button sizing standardization.The removal of
btn-smfrom both the visibility toggle and drag-handle buttons aligns with the PR's broader effort to establish a new default size for icon and circle buttons across approximately 40 templates.Also applies to: 39-39
projects/dashboards-demo/src/app/components/widget-catalog/custom-widget-catalog.component.html (1)
3-9: LGTM! Sizing change aligns with PR objectives.The removal of
btn-smfrom the close button is consistent with the PR's systematic consolidation of icon button sizing. The button structure is correct with proper accessibility attributes (aria-label="Close").Since this establishes a new default size for icon buttons, verify that the larger button size looks appropriate in the modal header UI.
projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.html (1)
10-12: LGTM! Formatting and size standardization look good.The single-line ternary improves readability, and the removal of
btn-smaligns with the PR's goal to establish a new default size for icon buttons.src/app/examples/si-notification-item/si-notification-item.html (3)
68-74: LGTM! Larger button improves touch targets.The removal of
btn-smestablishes the new default size for circle buttons and improves accessibility by providing a larger touch target for the approval action.
75-81: LGTM! Consistent sizing with approval button.The removal of
btn-smensures both approve and reject buttons have consistent sizing, maintaining visual harmony in the action pair.
110-116: LGTM! Consistent with sizing updates.The removal of
btn-smaligns this button with the new default sizing established throughout the notification item examples.projects/element-ng/action-modal/si-delete-confirmation-dialog/si-delete-confirmation-dialog.component.html (1)
7-14: Should this use the new squarebtn-iconvariant instead ofbtn-circle?Based on the PR objectives to introduce square
btn-iconas the new default for icon buttons, consider whether this modal cancel button should migrate frombtn-circletobtn-icon. Modal close buttons are a standard icon button use case and appear to fit the scope of the standardization effort.src/app/examples/si-modals/app-table.component.html (1)
6-6: LGTM! Size modifier removal is consistent with other modal close buttons.The change aligns with the broader pattern of removing
btn-smfrom modal close buttons throughout the codebase.src/app/examples/si-datepicker/si-modal-service.html (1)
9-9: LGTM! Consistent with the established pattern.The
btn-smremoval from this modal close button follows the same pattern as the other modal examples, ensuring visual consistency across the component library.src/app/examples/si-list-details/si-list-details-router.html (1)
23-23: The removal ofbtn-smis correct, but the suggestion to adoptbtn-iconappears to be beyond this PR's scope.This PR focuses on button sizing alignment (removing
btn-smand establishing new default sizes), which is properly applied here. However, the concern about convertingbtn-circletobtn-icon(square variant) conflates this with a separate styling concern. Whilebuttons.htmldemonstratesbtn-iconfor icon-only buttons, other example files throughout the codebase still usebtn-circlefor similar purposes, indicating both patterns coexist. Unless this file is specifically targeted for shape variant updates in this PR, thebtn-circlebuttons remain valid.projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (1)
91-91: LGTM: Button sizing standardization applied correctly.The removal of
btn-smaligns with the PR's goal of establishing a new default size for icon buttons.projects/element-ng/status-bar/si-status-bar.component.html (1)
110-110: LGTM: Size standardization applied as planned.The removal of
btn-smis consistent with the PR's incremental approach. As discussed in previous reviews, the migration to the newbtn-iconpattern will be handled in a separate PR.projects/element-ng/chat-messages/si-attachment-list.component.html (1)
47-47: LGTM: Consistent button sizing update.The removal of
btn-smcorrectly implements the size standardization for icon buttons across the attachment component.src/app/examples/buttons/buttoncategories.html (2)
12-19: LGTM: Navigation buttons sized for emphasis.Adding
btn-lgto the left/right navigation buttons creates appropriate visual hierarchy, distinguishing primary navigation actions from utility buttons.
24-36: LGTM: Utility buttons standardized to default size.The removal of
btn-smfrom the plus, delete, and options buttons establishes the new default sizing for utility icon buttons, creating a clear distinction from the larger navigation buttons.projects/element-ng/column-selection-dialog/si-column-selection-dialog.component.html (1)
5-5: LGTM: Modal close button sizing standardized.The removal of
btn-smcorrectly applies the new default sizing to the modal's close button, consistent with the button sizing standardization across the codebase.src/app/examples/si-list-details/si-list-details.html (1)
23-23: LGTM! Button sizing standardization applied correctly.The removal of
btn-smfrom both the options and filters buttons aligns with the PR's objective to establish a new default size for icon-only buttons.Also applies to: 41-41
projects/element-ng/action-modal/si-edit-discard-dialog/si-edit-discard-dialog.component.html (1)
9-9: LGTM! Modal header button sizing updated.The removal of
btn-smfrom the cancel button is consistent with the sizing refactor across the codebase.projects/element-ng/chat-messages/si-user-message.component.html (1)
19-19: LGTM! Chat message action buttons updated consistently.The removal of
btn-smfrom both the per-action buttons and the secondary actions menu button aligns with the PR's sizing standardization.Also applies to: 31-31
projects/element-ng/datepicker/components/si-calendar-direction-button.component.ts (1)
24-24: LGTM! Calendar direction buttons correctly use circular shape.The
btn-circle btn-tertiaryimplementation is correct per the design system guidelines, which explicitly state "Use circular shapes for icon-only buttons" (buttons.md, line 63). The removal ofbtn-smaligns with the sizing refactor.projects/element-ng/slider/si-slider.component.html (1)
4-4: Confirm whether slider buttons should usebtn-iconor remainbtn-circle.The buttons currently use
btn-circle, but the codebase showsbtn-iconis the standard pattern for icon-only buttons (used throughout buttons.html, button-groups.html, split-button.html, etc.). Since these slider buttons contain only icons with no text, verify whether they should be updated tobtn-iconto align with the standard, or if circular buttons are the intended design for slider controls.projects/element-ng/main-detail-container/si-main-detail-container.component.html (1)
94-94: LGTM! Sizing update aligns with PR objectives.The removal of
btn-smestablishes the new default size for circle buttons, consistent with the systematic sizing standardization across the codebase.projects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.html (1)
26-26: LGTM! Consistent sizing update.The removal of
btn-smfrom the action button aligns with the broader refactoring to standardize button sizes across components.docs/components/layout-navigation/modals.md (1)
82-82: LGTM! Documentation synchronized with implementation.The code example correctly reflects the new default button sizing, ensuring documentation stays consistent with the updated design system.
projects/element-ng/tree-view/si-tree-view.component.html (1)
4-4: LGTM! Expand/collapse buttons updated consistently.Both expand-all and collapse-all buttons have been updated to use the new default size, maintaining visual consistency in the tree view controls.
Also applies to: 12-12
src/app/examples/si-dashboard/si-list-widget-css.html (1)
20-20: LGTM! All list action buttons updated consistently.All six export buttons have been uniformly updated to use the new default size, ensuring visual consistency across the list widget example.
Also applies to: 31-31, 42-42, 53-53, 64-64, 75-75
projects/element-ng/file-uploader/si-file-uploader.component.html (1)
64-64: LGTM! Consistent removal ofbtn-smaligns with new default icon button sizing.The removal of the
btn-smmodifier from the retry, remove, and cancel buttons is consistent with the PR's objective to establish a new default size for icon buttons. The buttons retain all their functionality and semantic classes.Also applies to: 76-76, 87-87
projects/element-ng/chat-messages/si-ai-message.component.html (1)
16-16: LGTM! Button sizing update is consistent with the design system changes.The removal of
btn-smfrom both action buttons aligns with the standardized icon button sizing introduced in this PR. All functionality, accessibility attributes, and event handling remain intact.Also applies to: 28-28
projects/element-theme/src/styles/bootstrap/_buttons.scss (6)
11-18: LGTM! Well-structured icon button size variables.The new size and font-size variables for icon buttons are clearly named, consistently structured, and appropriately flagged with
!defaultfor customization. The size progression (24px → 28px → 32px → 40px) provides a coherent sizing scale.
154-163: LGTM! Excellent consolidation of shared icon button properties.The unified selector
:is(.btn-circle, .btn-icon, .btn-close):not(.btn-group .btn)effectively applies common styling (zero padding, inline-flex display, icon margin reset) across all icon button variants while correctly excluding button-group contexts where different rules apply. This reduces duplication and maintains clear separation of concerns.
165-171: LGTM! Correct semantic layering of icon button variants.The separate selectors establish
.btn-iconas the base variant (square border-radius) and.btn-circleas the specialized variant (50% border-radius). The source order ensures that combining both classes (class="btn btn-icon btn-circle") correctly results in a circular button, with.btn-circleoverriding the square radius. This implements the intended semantic hierarchy.
173-195: LGTM! Unified sizing ensures consistency across icon button variants.The combined sizing rules for
.btn-circleand.btn-iconensure both variants maintain consistent dimensions across all size modifiers (default, lg, sm, xs). Using thebutton-circle-sizemixin with the new$btn-icon-size-*variables provides a clean, maintainable approach that prevents sizing drift between square and circular icon buttons.
197-204: LGTM! Ghost styling correctly extended to new icon button variant.Adding
.btn-icon.btn-ghostto the selector ensures the new square icon buttons support the ghost variant styling, maintaining feature parity with.btn-circle.btn-ghost. This is essential for consistent theming across icon button shapes.
216-230: LGTM! Close button sizing updated for consistency with new icon button system.The
.btn-closenow uses the unified icon button sizing via thebutton-circle-sizemixin, ensuring visual consistency with.btn-circleand.btn-iconwhile preserving legacy compatibility styling (::before content and child element hiding). This maintains backward compatibility while aligning with the new design system.projects/element-ng/action-modal/si-alert-dialog/si-alert-dialog.component.html (1)
9-9: LGTM! Modal close button sizing updated consistently.Removing
btn-smfrom the modal header close button aligns with the standardized icon button sizing. All functionality and accessibility attributes are preserved.projects/element-ng/navbar-vertical/si-navbar-vertical.component.html (1)
20-20: LGTM! Navigation collapse button sizing updated consistently.Removing
btn-smfrom the navbar collapse toggle button aligns with the new default icon button sizing established in this PR. The button's functionality, accessibility, and state management remain unchanged.projects/element-ng/threshold/si-threshold.component.html (2)
6-13: LGTM: Button sizing standardization applied correctly.The removal of
btn-smaligns with the PR's goal to establish a new default size for icon-only and circle buttons.
51-59: LGTM: Consistent with sizing standardization.The removal of
btn-smis consistent with the delete button above and the overall PR objective.projects/element-ng/side-panel/si-side-panel-content.component.html (5)
16-35: LGTM: Navigation button sizing updated.The removal of
btn-smfrom the router-link anchor is consistent with the button sizing standardization.
37-50: LGTM: Consistent with router-link button above.
54-66: LGTM: Fullscreen button sizing standardized.
68-76: LGTM: Close button sizing updated.
78-89: LGTM: Collapse toggle sizing standardized.projects/element-ng/chat-messages/si-chat-input.component.html (4)
51-59: LGTM: Attachment button sizing standardized.
65-73: LGTM: Action buttons sizing updated.
77-85: LGTM: Secondary actions button sizing standardized.
98-106: LGTM: Main action button sizing updated.src/app/examples/si-chat-messages/si-attachment-list.html (1)
18-24: LGTM: Modal close button sizing standardized.src/app/examples/datatable/datatable-grouped-rows.html (1)
19-27: LGTM: Group toggle button sizing standardized.The removal of
btn-smis consistent with the project-wide button sizing refactoring.src/app/examples/si-chat-messages/si-chat-container.html (2)
31-31: LGTM!The standardized button sizing for chat message actions is consistent with the PR-wide refactoring.
80-80: LGTM!Modal close button sizing standardization is consistent with similar changes across other modal templates in the PR.
projects/element-ng/photo-upload/si-photo-upload.component.html (1)
64-64: LGTM!The modal close button sizing standardization is consistent with the PR-wide effort to establish default sizing for icon buttons.
src/app/examples/si-chat-messages/si-chat-input.html (1)
27-27: LGTM!The emoji picker button sizing standardization aligns with the broader effort to establish consistent default sizing for icon buttons across the chat UI.
src/app/examples/si-chat-messages/si-chat-message.html (1)
11-11: LGTM!The consistent removal of
btn-smfrom all chat message action buttons standardizes their sizing across different message alignments and action positions. This aligns with the PR-wide button sizing refactoring.Also applies to: 22-22, 43-43
projects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.html (1)
34-34: Button size standardization is consistent across toast files.The removal of
btn-smis complete with no remaining instances in toast-related files. The close button classes (btn btn-circle btn-ghost) are consistent, and the negative margins (mt-n2 me-n2) remain in place to manage spacing for the default-sized button.Visual verification in the toast context would be beneficial to confirm the larger button does not cause layout overflow with these negative margins.
src/app/examples/datatable/bootstrap.html (1)
41-41: LGTM! Verify table layout with larger button size.The removal of
btn-smfrom these circle dropdown buttons aligns with the PR's goal of establishing a new default size for icon buttons. Ensure that the larger button size doesn't negatively impact the table's visual hierarchy or spacing.Also applies to: 75-75, 104-104, 133-133
projects/element-ng/notification-item/si-notification-item.component.html (1)
112-112: LGTM! Consistent size standardization across notification action buttons.The removal of
btn-smfrom all notification item action buttons (action buttons, router links, links, and menu triggers) is consistent with the PR's goal of standardizing icon button sizing.Also applies to: 122-122, 140-140, 152-152
src/app/examples/buttons/buttons.html (2)
47-214: Icon button section ordering aligns with PR feedback.The restructure correctly positions square icon buttons (lines 47-127) before circle buttons (lines 129-214), which aligns with kfenner's recommendation in the PR objectives that square buttons should be the new default icon buttons.
Based on PR objectives feedback from kfenner.
223-273: Icon button sizing examples answer thexsvariant question.The Icon button sizing section demonstrates all size variants (lg, default, sm, xs) for both square and circle icon buttons, including the
xsvariant that kfenner asked about in previous comments (lines 263, 269).Given kfenner's earlier concern that icon button sizing is "utterly confusing" and needs resolution for v49, and dauriamarco's reference to issue #1222 for future alignment, this section provides comprehensive examples but may benefit from the planned sizing consolidation.
projects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.html (1)
27-84: Verify whether buttons should follow the documentedbtn-iconpattern.The timeline widget action buttons use
btn-circlefor icon-only buttons, which is consistent across the element-ng codebase. However, the official button documentation (button-group.md) specifiesbtn-iconas the standard class for icon-only buttons. Clarify whetherbtn-circleis an intentional alternative for this component context or whether these buttons should be updated to usebtn-iconto align with the documented button pattern.Likely an incorrect or invalid review comment.
dauriamarco
added
the
breaking-changes
dauriamarco
force-pushed
the
feat/btn-icon
branch
from
46d7a09 to
9aab76d
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.html (1)
24-33: Consider migrating to the square.btn-iconvariant and verify manual padding.Per the PR objectives, square buttons (
.btn-icon) should be the new default for icon buttons, with.btn-circlereserved for cases where a circular shape is specifically needed. Additionally, the manualp-2padding class may be redundant or conflict with the new unified sizing system introduced in this PR.Consider:
- Replacing
btn-circlewithbtn-iconto align with the new default, unless a circular button is specifically required here- Verifying whether the
p-2padding is still needed, as the unified.btn-iconsizing rules may already handle padding appropriately🔄 Suggested migration to square button
<button type="button" - class="btn btn-circle btn-tertiary ms-4 p-2" + class="btn btn-icon btn-tertiary ms-4" [attr.aria-label]="valueObject.action.title | translate" [siLink]="valueObject.action" >
🤖 Fix all issues with AI agents
In @projects/element-ng/wizard/si-wizard.component.html:
- Line 124: Replace the deprecated circular icon button class with the new
square default: change the element that currently uses "btn-circle" (the icon
button in si-wizard.component.html) to use "btn-icon" instead, keeping the
existing "btn btn-secondary mt-2 mb-4" classes intact so sizing and spacing
remain consistent.
In @src/app/examples/buttons/buttoncategories.html:
- Around line 24-36: Add a square icon button example next to the existing
circle examples by including a button element using the new .btn-icon variant
(e.g., class="btn btn-icon btn-secondary element-plus" with appropriate
aria-label) so the demo shows both .btn-circle and .btn-icon; ensure the new
button follows the same structure and margin classes (like me-6) and uses the
existing element-* utility classes so styling and semantics match the other
examples.
In @src/app/examples/si-modals/app-table.component.html:
- Line 6: Replace the obsolete rounded class on the modal close button: in the
element with class="btn btn-circle btn-ghost element-cancel" remove btn-circle
and add btn-icon so it becomes btn btn-icon btn-ghost element-cancel; keep the
existing btn, btn-ghost and element-cancel classes intact to preserve styling
and behavior.
In @src/app/examples/si-modals/si-modal-service.html:
- Line 32: Replace the close-button class list that currently reads "btn
btn-circle btn-ghost element-cancel" (present in the three modal header close
buttons) with the new standardized square icon class "btn btn-icon btn-ghost
element-cancel"; update all three occurrences so the close buttons use .btn-icon
instead of .btn-circle/.btn-sm to match the PR's sizing consolidation.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (10)
playwright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-no-backdrop-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-no-backdrop-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--scroll-mode-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--scroll-mode-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel-collapsible--open-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel-collapsible--open-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (50)
docs/components/layout-navigation/modals.mdplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons.yamlprojects/dashboards-demo/src/app/components/widget-catalog/custom-widget-catalog.component.htmlprojects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.htmlprojects/element-ng/action-modal/si-alert-dialog/si-alert-dialog.component.htmlprojects/element-ng/action-modal/si-confirmation-dialog/si-confirmation-dialog.component.htmlprojects/element-ng/action-modal/si-delete-confirmation-dialog/si-delete-confirmation-dialog.component.htmlprojects/element-ng/action-modal/si-edit-discard-dialog/si-edit-discard-dialog.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.htmlprojects/element-ng/column-selection-dialog/si-column-selection-dialog.component.htmlprojects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/file-uploader/si-file-uploader.component.htmlprojects/element-ng/main-detail-container/si-main-detail-container.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/notification-item/si-notification-item.component.htmlprojects/element-ng/photo-upload/si-photo-upload.component.htmlprojects/element-ng/side-panel/si-side-panel-content.component.htmlprojects/element-ng/slider/si-slider.component.htmlprojects/element-ng/status-bar/si-status-bar.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.htmlprojects/element-ng/tour/si-tour.component.htmlprojects/element-ng/tree-view/si-tree-view.component.htmlprojects/element-ng/wizard/si-wizard.component.htmlprojects/element-theme/src/styles/bootstrap/_button-group.scssprojects/element-theme/src/styles/bootstrap/_buttons.scsssrc/app/examples/buttons/buttoncategories.htmlsrc/app/examples/buttons/buttons.htmlsrc/app/examples/datatable/bootstrap.htmlsrc/app/examples/datatable/datatable-grouped-rows.htmlsrc/app/examples/si-chat-messages/si-attachment-list.htmlsrc/app/examples/si-chat-messages/si-chat-container.htmlsrc/app/examples/si-chat-messages/si-chat-input.htmlsrc/app/examples/si-chat-messages/si-chat-message.htmlsrc/app/examples/si-dashboard/si-list-widget-css.htmlsrc/app/examples/si-datepicker/si-modal-service.htmlsrc/app/examples/si-list-details/si-list-details-router.htmlsrc/app/examples/si-list-details/si-list-details.htmlsrc/app/examples/si-map/si-map-custom-cluster-popover.tssrc/app/examples/si-modals/app-table.component.htmlsrc/app/examples/si-modals/si-modal-service.htmlsrc/app/examples/si-notification-item/si-notification-item.htmlsrc/app/examples/si-wizard/si-wizard-forms.html
🧰 Additional context used
🧠 Learnings (18)
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.htmlprojects/element-ng/status-bar/si-status-bar.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/notification-item/si-notification-item.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tssrc/app/examples/buttons/buttons.htmlprojects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/tour/si-tour.component.html
📚 Learning: 2025-12-22T13:04:35.578Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:35.578Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
projects/element-ng/file-uploader/si-file-uploader.component.htmlprojects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.htmlprojects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.htmlprojects/element-ng/status-bar/si-status-bar.component.htmlprojects/element-ng/wizard/si-wizard.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/notification-item/si-notification-item.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/tree-view/si-tree-view.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/tour/si-tour.component.html
📚 Learning: 2025-12-09T14:31:33.543Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:14-50
Timestamp: 2025-12-09T14:31:33.543Z
Learning: In projects/element-ng/chat-messages/si-ai-chat-container.component.html, message IDs can change, so tracking by message.id is not appropriate. The component uses `track message` (object identity) which is the correct approach for this use case.
Applied to files:
src/app/examples/si-chat-messages/si-chat-message.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlsrc/app/examples/si-chat-messages/si-chat-container.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2025-12-22T13:16:19.243Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/icon/element-icons.ts:166-167
Timestamp: 2025-12-22T13:16:19.243Z
Learning: In projects/element-ng/icon/element-icons.ts, the elementCheckedImageShape icon intentionally uses base64 encoding instead of utf8 encoding (which is used for other icons). This approach is considered safer and correct for this specific icon and should not be flagged as an inconsistency.
Applied to files:
projects/element-ng/status-bar/si-status-bar.component.html
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/side-panel/si-side-panel-content.component.html
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
projects/element-ng/datepicker/components/si-calendar-direction-button.component.tssrc/app/examples/si-map/si-map-custom-cluster-popover.ts
📚 Learning: 2025-12-30T13:52:33.581Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:33.581Z
Learning: Maintain the existing negative naming convention (e.g., disableBackdrop) across the siemens/element repository for consistency. Do not refactor to positive names (e.g., showBackdrop, hasBackdrop) unless the project explicitly changes the convention. In reviews, flag changes that alter established naming patterns in related files under projects/element-ng.
Applied to files:
projects/element-ng/datepicker/components/si-calendar-direction-button.component.ts
📚 Learning: 2025-12-09T14:32:26.166Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:21-47
Timestamp: 2025-12-09T14:32:26.166Z
Learning: In Angular templates, prefer using nested *ngIf blocks over *ngSwitch for branching on message types within the chat-messages components. Use *ngIf with else branches to handle distinct cases and avoid complex switch logic in HTML templates. This guideline applies to all HTML templates under projects/element-ng/chat-messages.
Applied to files:
projects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2025-11-18T12:37:30.510Z
Learnt from: Killusions
Repo: siemens/element PR: 1040
File: projects/element-ng/chat-messages/si-chat-input.component.ts:338-349
Timestamp: 2025-11-18T12:37:30.510Z
Learning: In projects/element-ng/chat-messages/si-chat-input.component.ts, the interrupt behavior is intentionally different for button clicks vs Enter key: the button can emit interrupt even without content, but pressing Enter requires content or attachments (canSend() must be true) to emit interrupt.
Applied to files:
projects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-map/si-map-custom-cluster-popover.ts
📚 Learning: 2026-01-07T08:57:45.608Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1226
File: src/app/examples/si-navbar-vertical/si-navbar-vertical.ts:112-115
Timestamp: 2026-01-07T08:57:45.608Z
Learning: Do not strictly enforce badge color consistency in example files under src/app/examples. Treat badge color checks as non-blocking for TypeScript files in this directory; reviewers may still note inconsistencies, but they should not require changes for these example files. Apply this guidance broadly to all files within src/app/examples.
Applied to files:
src/app/examples/si-map/si-map-custom-cluster-popover.ts
📚 Learning: 2026-01-07T08:57:45.608Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1226
File: src/app/examples/si-navbar-vertical/si-navbar-vertical.ts:112-115
Timestamp: 2026-01-07T08:57:45.608Z
Learning: Badge color consistency in example files (src/app/examples/) is not a priority and should not be strictly enforced during reviews.
Applied to files:
src/app/examples/buttons/buttons.html
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
src/app/examples/buttons/buttons.html
📚 Learning: 2026-01-02T16:41:57.274Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:57.274Z
Learning: In Element documentation (siemens/element), sizing utility examples in docs may intentionally differ from implementation examples to demonstrate multiple valid approaches - for example, documentation might use traditional CSS properties like `height` and `width` while the implementation example uses logical properties like `block-size` and `inline-size`. These are considered different examples rather than inconsistencies.
Applied to files:
src/app/examples/buttons/buttons.html
📚 Learning: 2025-12-01T09:34:50.469Z
Learnt from: Killusions
Repo: siemens/element PR: 1047
File: docs/fundamentals/ux-text-style-guide/basics.md:30-30
Timestamp: 2025-12-01T09:34:50.469Z
Learning: In docs/fundamentals/ux-text-style-guide/basics.md, the h4 heading level (####) for "Do's" and "Don'ts" within the dos-and-donts structure is intentional, even though it triggers the MD001 linting rule about heading hierarchy.
Applied to files:
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons.yaml
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons.yaml
📚 Learning: 2026-01-02T16:41:48.359Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:48.359Z
Learning: In Element docs, sizing examples may differ between documentation and implementation to illustrate multiple valid approaches. Do not flag such differences as inconsistencies; treat them as separate illustrative examples (e.g., using height/width in docs vs block-size/inline-size in implementation). Apply this understanding across Markdown documentation files to avoid false-positive review flags.
Applied to files:
docs/components/layout-navigation/modals.md
📚 Learning: 2025-12-22T13:29:36.922Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/ag-grid/parts/color-scheme.part.ts:35-35
Timestamp: 2025-12-22T13:29:36.922Z
Learning: In the Element design system (projects/element-ng/theme and projects/element-theme), only `--element-body-font-family` is exposed as a CSS custom property for typography. Font sizes are NOT exposed as CSS custom properties; they are defined using internal SCSS variables (e.g., `typography.$si-font-size-body`) that compile to fixed values. Using hardcoded pixel values for fontSize in AG Grid theme configuration (e.g., `fontSize: '14px'`) is the correct approach.
Applied to files:
projects/element-theme/src/styles/bootstrap/_buttons.scss
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (50)
projects/element-ng/threshold/si-threshold.component.html (1)
8-8: LGTM — straightforward sizing standardization.Both buttons have the
btn-smmodifier removed as part of the PR's consolidation of icon/button sizing. The buttons remain styled asbtn-circleand retain their accessibility attributes (aria-labels, disabled state binding). The removal ofbtn-smaligns with the new consolidated sizing rules introduced in the PR's SCSS changes (e.g.,_button-group.scssand_buttons.scss).Also applies to: 53-53
src/app/examples/si-map/si-map-custom-cluster-popover.ts (1)
26-26: LGTM! Aligns with button styling consolidation.The removal of
btn-smis consistent with the PR-wide effort to standardize icon button sizing and introduce the new square button variant as the default.src/app/examples/si-dashboard/si-list-widget-css.html (1)
20-20: LGTM! Consistent removal of btn-sm aligns with the icon button sizing refactor.The removal of
btn-smfrom all six action buttons is uniform and aligns with the PR's objective to consolidate icon/button styling. These buttons will now adopt the standardized icon button sizing introduced by the new.btn-iconrules and$btn-icon-size-*variables mentioned in the AI summary.Also applies to: 31-31, 42-42, 53-53, 64-64, 75-75
projects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.html (1)
29-29: Consider adopting the new square button variant (btn-icon).Both buttons have had
btn-smremoved as part of the unified icon button sizing strategy. However, they still usebtn-circle. Given kfenner's comment that "square buttons should be the new default icon buttons," should these be updated to use the newbtn-iconclass instead ofbtn-circlefor consistency with the new default?If the design intent is to keep certain buttons as circles, this change is fine. However, please verify whether these icon-only buttons should adopt the square variant to align with the new defaults.
Also applies to: 39-39
projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.html (1)
11-11: LGTM! Verify the visual appearance of the icon button.The removal of
btn-smaligns with the broader icon-button sizing standardization across this PR. The edit button will now render at the default icon button size when displayed as an icon-only button (showEditButtonLabelDesktopis false).Please verify that the larger button size integrates well with the toolbar layout and matches the intended design for the new default icon buttons.
projects/element-ng/tree-view/si-tree-view.component.html (1)
3-18: LGTM!The removal of
btn-smfrom both expand and collapse buttons aligns with the PR's broader consolidation of icon/button sizing. Both buttons maintain proper accessibility attributes (title,aria-label) and event handling, with no functional regressions.Please verify that the buttons' visual sizing in the tree-view header remains appropriate after removing the
btn-smmodifier. The SCSS consolidation introduced in this PR should provide the correct sizing via the updated.btn-circlerules, but confirm that the expand/collapse buttons display at the expected size in the UI.projects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.html (1)
81-81: Removal ofbtn-smaligns with PR's button sizing consolidation.The removal of the
[class.btn-sm]="action.iconOnly"binding from the menu button is consistent with this PR's objective to standardize icon button sizing across components. The button will now use the default.btn-circlesize.Please verify that the menu button's visual appearance and size remain appropriate in the timeline widget UI without the small size modifier.
projects/element-ng/file-uploader/si-file-uploader.component.html (3)
62-71: Consider using the new squarebtn-iconvariant.Based on the PR objectives and kfenner's comment that square buttons should be the new default icon buttons, consider whether this retry button should use the new square
btn-iconclass instead ofbtn-circle.Additionally, verify that the visual size after removing
btn-smis appropriate for the file uploader UI.
73-83: Consider using the new squarebtn-iconvariant.Based on the PR objectives and kfenner's comment that square buttons should be the new default icon buttons, consider whether this remove button should use the new square
btn-iconclass instead ofbtn-circle.Additionally, verify that the visual size after removing
btn-smis appropriate for the file uploader UI.
84-94: Consider using the new squarebtn-iconvariant.Based on the PR objectives and kfenner's comment that square buttons should be the new default icon buttons, consider whether this cancel button should use the new square
btn-iconclass instead ofbtn-circle.Additionally, verify that the visual size after removing
btn-smis appropriate for the file uploader UI.projects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.html (1)
31-40: The button styling is correct. Close buttons throughout the component library consistently usebtn-circle, and the removal ofbtn-smaligns with the PR's standardization effort. There is no evidence from the codebase that close buttons should be converted to thebtn-iconsquare variant—similar close buttons insi-tab.component.html,si-input-pill.component.html, and other components remain as circular buttons.Likely an incorrect or invalid review comment.
projects/element-ng/side-panel/si-side-panel-content.component.html (1)
17-17: No changes needed. These buttons are intentionally circular (btn-circle) as part of the side panel's UI design. The PR's focus on button sizing alignment (removingbtn-sm) is separate from shape variants. Thebtn-iconclass andbtn-circleclass serve different purposes and are not mutually exclusive—examples in the codebase show both used together when circular icon buttons are needed.Likely an incorrect or invalid review comment.
projects/element-ng/action-modal/si-delete-confirmation-dialog/si-delete-confirmation-dialog.component.html (1)
7-14: No action required; the button styling is correct as-is.The removal of
btn-smaligns with the button sizing refactoring and is appropriate. Modal close buttons should usebtn-circle(as shown in the official modal documentation), notbtn-icon. Thebtn-iconclass is documented for icon-only button groups and has a different visual style (square with semantic border-radius) compared tobtn-circle(fully circular). The current implementation is consistent with the documented best practice for modal close buttons.Likely an incorrect or invalid review comment.
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (1)
91-91: LGTM! Consistent removal ofbtn-smfor icon button standardization.The removal of
btn-smfrom this close button aligns with the PR's goal to consolidate and standardize icon button sizing across the component library.projects/element-ng/datepicker/components/si-calendar-direction-button.component.ts (1)
24-24: LGTM! Icon button sizing standardization applied correctly.The removal of
btn-smfrom the calendar navigation button is consistent with the broader UI cleanup in this PR.projects/element-ng/chat-messages/si-attachment-list.component.html (1)
47-47: LGTM! Consistent styling update for attachment remove button.The removal of
btn-smaligns with the PR's icon button standardization effort.projects/element-ng/status-bar/si-status-bar.component.html (1)
110-110: LGTM! Icon button sizing standardization applied.The removal of
btn-smfrom the mute button is consistent with this PR's goal. The planned migration frombtn-circletobtn-iconwill be addressed in the follow-up PR as previously discussed.projects/element-ng/navbar-vertical/si-navbar-vertical.component.html (1)
20-20: LGTM! Consistent button sizing update.The removal of
btn-smfrom the navbar collapse toggle button aligns with the icon button standardization effort in this PR.src/app/examples/buttons/buttoncategories.html (1)
12-19: LGTM! Sizing adjustments align with new button system.The addition of
btn-lgto the navigation buttons correctly demonstrates the updated sizing options for circular icon buttons.src/app/examples/si-datepicker/si-modal-service.html (1)
9-9: Consistent button sizing standardization.The removal of
btn-smaligns with the PR's goal to standardize icon button sizing. Ensure the close button's visual appearance matches the intended design after the CSS changes.src/app/examples/si-notification-item/si-notification-item.html (1)
71-71: LGTM: Notification action buttons updated consistently.The removal of
btn-smfrom notification action buttons (approve, reject, and view details) is consistent with the PR's button sizing standardization. The changes maintain all functional attributes and event handlers.Also applies to: 78-78, 113-113
projects/element-ng/column-selection-dialog/si-column-selection-dialog.component.html (1)
5-5: Consistent modal header button standardization.The removal of
btn-smfrom the cancel button aligns with the broader refactoring effort. The functional behavior and accessibility attributes remain unchanged.projects/element-ng/photo-upload/si-photo-upload.component.html (1)
64-64: Modal cancel button sizing updated consistently.The removal of
btn-smfrom the photo upload modal's cancel button follows the established pattern across the codebase. All functional and accessibility attributes are preserved.projects/element-ng/action-modal/si-edit-discard-dialog/si-edit-discard-dialog.component.html (1)
9-9: Edit discard dialog cancel button updated consistently.The
btn-smremoval from the cancel button is consistent with the PR's standardization effort. The button's functional attributes, including the loading state handling and event binding, remain intact.projects/element-ng/chat-messages/si-user-message.component.html (1)
19-19: LGTM: Consistent sizing for chat message action buttons.The removal of
btn-smfrom both action buttons standardizes the sizing with the new unified icon/button approach. The chat message action context has adequate space for the default button size.Also applies to: 31-31
projects/element-ng/main-detail-container/si-main-detail-container.component.html (1)
94-94: LGTM: Improved touch target for mobile navigation.The removal of
btn-smfrom the back button aligns with the sizing standardization and provides an accessibility benefit—larger touch targets are especially valuable for this navigation button that appears on smaller screens.src/app/examples/si-chat-messages/si-attachment-list.html (1)
20-20: LGTM: Standardized modal close button sizing.The removal of
btn-smfrom the modal close button is consistent with the sizing standardization and aligns with modal patterns across the codebase.projects/element-ng/action-modal/si-confirmation-dialog/si-confirmation-dialog.component.html (1)
9-9: LGTM: Consistent modal close button pattern.The removal of
btn-smfrom the confirmation dialog close button maintains consistency with other modal close buttons in the codebase and aligns with the standardized sizing approach.src/app/examples/si-list-details/si-list-details.html (1)
23-23: Visual regression tests confirm the sizing change is safe.The Playwright E2E tests for this component include comprehensive visual regression snapshots (
si.runVisualAndA11yTests()) covering the default layout, small viewport (600px), and resized scenarios. The larger default button size works well in the header contexts on both lines 23 and 41, and the test coverage confirms no overflow or spacing issues occur, including on smaller viewports.docs/components/layout-navigation/modals.md (1)
82-82: Documentation updated to reflect new button sizing standards.The removal of
btn-smfrom the modal cancel button example aligns with the PR's standardization of icon button sizing. The new unified.btn-iconand.btn-circlesizing system handles the appropriate sizing automatically.projects/element-ng/chat-messages/si-chat-input.component.html (1)
53-53: LGTM! Button sizing standardization applied consistently.All four icon buttons have been updated to use the new standardized sizing by removing the
btn-smmodifier. The unified.btn-circlesizing system will handle the appropriate dimensions automatically.Also applies to: 67-67, 79-79, 100-100
src/app/examples/si-list-details/si-list-details-router.html (1)
23-23: Example updated to demonstrate new button sizing standards.Both icon buttons correctly demonstrate the new unified sizing approach by removing
btn-sm. The.btn-circleclass now provides consistent sizing automatically.Also applies to: 41-41
src/app/examples/datatable/bootstrap.html (1)
41-41: Datatable example updated with consistent button sizing.All four expand/dropdown buttons now use the standardized sizing. The removal of
btn-smensures consistency with the new.btn-circlesizing rules across the table interface.Also applies to: 75-75, 104-104, 133-133
projects/element-ng/chat-messages/si-ai-message.component.html (1)
16-16: Chat message actions updated with standardized button sizing.Both action buttons now use the unified sizing system. The removal of
btn-smis consistent with the changes insi-chat-input.component.htmland ensures visual consistency across all chat components.Also applies to: 28-28
projects/element-ng/notification-item/si-notification-item.component.html (1)
112-112: LGTM! Consistent removal ofbtn-smaligns with the new sizing strategy.The removal of
btn-smfrom all icon button variants is consistent with the broader consolidation of button sizing across the codebase. All functional attributes, event handlers, and accessibility labels remain intact.Also applies to: 122-122, 140-140, 152-152
projects/element-theme/src/styles/bootstrap/_buttons.scss (5)
11-18: LGTM! Well-structured size variable definitions.The new
$btn-icon-*size variables establish a clear progression (24px → 28px → 32px → 40px) and provide appropriate font sizing for icon buttons. The naming convention is consistent and aligns with the existing button size modifiers.
154-163: LGTM! Effective consolidation of icon button styles.The unified selector
:is(.btn-circle, .btn-icon, .btn-close):not(.btn-group .btn)successfully consolidates shared styling while correctly excluding button-group contexts where specialized sizing applies. The exclusion is necessary because_button-group.scssdefines custom.btn-iconpadding rules for group layouts.
165-171: LGTM! Border-radius cascade correctly implements the design intent.The CSS order positions
.btn-icon(square with$element-button-radius) before.btn-circle(50% radius), allowing the combinationclass="btn btn-icon btn-circle"to produce a circular button. This matches the semantic model discussed in past reviews where.btn-iconis the base and.btn-circleis a specialization.
173-195: LGTM! Sizing consolidation eliminates duplication.The shared sizing block for
.btn-circleand.btn-iconeffectively centralizes dimension logic using the new$btn-icon-size-*variables. This ensures both variants maintain consistent sizing across all modifiers (default, lg, sm, xs).
197-204: LGTM! Complete integration of.btn-iconinto existing button variants.The addition of
.btn-icon.btn-ghostat line 198 and the update of.btn-closesizing to use$btn-icon-size-defaultat line 218 ensure the new icon button system is fully integrated with existing button types. This maintains visual consistency across all icon-based button variants.Also applies to: 218-218
projects/element-theme/src/styles/bootstrap/_button-group.scss (1)
55-58: LGTM! Appropriate button-group override for.btn-icon.This rule correctly provides the necessary override for
.btn-iconelements within.btn-group, which are excluded from the main icon button styles via:not(.btn-group .btn)in_buttons.scss. The padding calculation follows the established button-group pattern and accounts for border widths appropriately.src/app/examples/datatable/datatable-grouped-rows.html (1)
21-21: LGTM! Consistent with the sizing consolidation strategy.The removal of
btn-smfrom the group expansion toggle button aligns with the PR's objective to standardize icon button sizing. The button retains all functional attributes and accessibility properties.projects/element-ng/slider/si-slider.component.html (1)
4-4: LGTM! Slider button sizing aligned with new standards.The removal of
btn-smfrom both the decrement and increment buttons is consistent with the PR's sizing consolidation. All event handlers, disabled states, and ARIA attributes are correctly preserved.Also applies to: 62-62
projects/element-ng/action-modal/si-alert-dialog/si-alert-dialog.component.html (1)
9-9: LGTM - Aligned with the new button sizing system.The removal of
btn-smis consistent with the PR's unified button sizing approach, maintaining visual consistency across the component library.projects/dashboards-demo/src/app/components/widget-catalog/custom-widget-catalog.component.html (1)
5-5: LGTM - Consistent with button sizing standardization.The change aligns with the PR's unified button sizing system.
projects/element-ng/tour/si-tour.component.html (1)
32-32: LGTM - Button sizing correctly aligned.The change maintains consistent sizing as confirmed in previous review discussions.
src/app/examples/si-chat-messages/si-chat-input.html (1)
27-27: LGTM - Sizing standardization applied correctly.The removal of
btn-smaligns with the unified button sizing approach across the example components.src/app/examples/si-chat-messages/si-chat-message.html (1)
11-11: LGTM - Consistent button sizing applied across chat message actions.All three instances correctly remove
btn-smin alignment with the PR's unified button sizing system.Also applies to: 22-22, 43-43
src/app/examples/si-chat-messages/si-chat-container.html (1)
31-31: LGTM! Removal ofbtn-smaligns with sizing consolidation.The removal of the
btn-smmodifier from these buttons aligns with the PR's consolidation of button sizing across the codebase. The new unified sizing approach for icon/circle buttons provides consistency.Also applies to: 80-80
src/app/examples/buttons/buttons.html (1)
1-46: LGTM! Excellent restructuring and section organization.The reorganization into clearly labeled sections with descriptive headings significantly improves the discoverability of button variants. The ordering—with icon buttons (square) appearing before circle buttons—correctly implements the recommendation that square icon buttons should be the default.
Also applies to: 129-222, 275-308
dauriamarco
force-pushed
the
feat/btn-icon
branch
from
9aab76d to
7a73280
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
src/app/examples/si-wizard/si-wizard-forms.html (1)
6-12: Verify modal-header layout after removingbtn-sm; consider switching to.btn-iconif square is now the default.Dropping
btn-smlooks consistent with the PR-wide move to unified icon sizing, but please confirm the modal header doesn’t grow/shift and the close affordance still renders correctly (this button has no inner content and likely relies on.element-cancelstyling). If this close button is meant to reflect the new default icon-button style, consider updating frombtn-circleto the new.btn-iconvariant instead.projects/element-ng/notification-item/si-notification-item.component.html (4)
109-118: Confirm these icon actions should remain.btn-circle(vs new default.btn-icon) and verify sizing after droppingbtn-sm.
Given the PR direction (“square btn-icon”), consider switching this to.btn-iconunless the circular affordance is intentional for notification quick actions. Also verify the clickable area remains consistent (esp. if$btn-icon-size-*now drives sizing).
120-136:p-2may conflict with the new unified icon sizing—verify this doesn’t oversize the control.
If icon sizing is now standardized via.btn-circle/.btn-iconrules, consider removing manual padding and/or moving to.btn-iconfor the new square default.
138-147: Same concern as routerLink: validate padding + icon sizing consistency.
If the new sizing system is intended to eliminate template-level padding tweaks, dropp-2and rely on the icon-button class.
149-158: A11y/i18n: avoid concatenating English suffixes toheading()inaria-label.
Ifheading()is a translation key (it’s piped throughtranslateelsewhere), the current label can become non-human-readable and is not localized. Prefer a translated full string (or a translation key with interpolation).Proposed minimal template fix (if acceptable)
- [attr.aria-label]="heading() + ' dropdown'" + [attr.aria-label]="(heading() | translate) + ' dropdown'"projects/element-ng/filtered-search/si-filtered-search.component.html (1)
66-72: Addbtn-iconbase class to icon-only buttons.These buttons lack the
btn-iconbase class used consistently across all icon-only buttons in the design system. Update lines 66 and 79 to use the patternbtn btn-icon btn-sm(for square shape) orbtn btn-icon btn-circle btn-sm(if circular shape is required). The examples throughout the codebase show thatbtn-iconis the required foundation, withbtn-circleas an optional modifier.Current patterns:
- Line 66:
class="btn btn-circle btn-sm btn-ghost mx-2"- Line 79:
class="search-button btn btn-circle btn-sm btn-secondary focus-inside"Should be:
class="btn btn-icon btn-sm btn-ghost mx-2"(and similar for line 79)Or if circular shape is intentional:
class="btn btn-icon btn-circle btn-sm btn-ghost mx-2"projects/element-theme/src/styles/bootstrap/_button-group.scss (1)
14-18: Likely layout bug:.btn-iconwill still stretch inside.btn-groupdue toflex: 1 1 auto.Right now
.btn-iconmatches.btn:not(.btn-circle)(Line 14), so it will expand like a regular button even though the new block (Line 55-58) looks aimed at icon-only sizing. Consider excluding.btn-iconfrom the flexing selector and explicitly opting it out of flex.Proposed fix
.btn-group { @@ - .btn:not(.btn-circle) { + .btn:not(.btn-circle, .btn-icon) { position: relative; flex: 1 1 auto; border-radius: 0; } @@ - > .btn:first-of-type:not(.btn-circle), - > label:first-of-type .btn:not(.btn-circle) { + > .btn:first-of-type:not(.btn-circle, .btn-icon), + > label:first-of-type .btn:not(.btn-circle, .btn-icon) { border-start-start-radius: semantic-tokens.$element-button-radius; border-end-start-radius: semantic-tokens.$element-button-radius; } @@ - > .btn:last-of-type:not(.btn-circle), - > label:last-of-type .btn:not(.btn-circle) { + > .btn:last-of-type:not(.btn-circle, .btn-icon), + > label:last-of-type .btn:not(.btn-circle, .btn-icon) { border-start-end-radius: semantic-tokens.$element-button-radius; border-end-end-radius: semantic-tokens.$element-button-radius; } @@ .btn-icon { + flex: 0 0 auto; inline-size: unset; padding-inline: calc(#{map.get(spacers.$spacers, 2)} - var(--btn-border-width, 0px)); } }Also applies to: 24-34, 55-58
projects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.html (1)
79-85: Icon will render via theelement-options-verticalweb font class, but consider adding a visual test to verify alignment remains stable after the size change (24px → 32px).The button's icon is properly styled through CSS; however, the component lacks visual regression test coverage for this action button. While the icon will display, a snapshot test would ensure the increased button size (from
btn-smto default) doesn't affect row height or spacing in the timeline widget layout.projects/element-ng/tabs/si-tab.component.html (1)
9-14: Implement proper button semantics for close button.The close button applies button CSS classes directly to the
si-iconcomponent rather than using a<button>element, creating accessibility issues:
- Keyboard accessibility missing: The click handler lacks keyboard support. While the parent tab handles Enter key, the close button has no keyboard event binding (compare to tab's
(keydown.enter)pattern)- No button semantics: CSS classes alone don't provide button role or semantics;
si-iconis a presentational component that consumers must properly contextualize- Screen reader issues: No button role means assistive technologies won't announce this as an interactive control
Wrap
si-iconin a<button>element with appropriate attributes, or add keyboard event handling androle="button"if a button element is not feasible.projects/element-ng/navbar-vertical/si-navbar-vertical.component.html (1)
18-31: Confirm intended shape: keep.btn-circlevs migrate to new.btn-icon.Line 20 removes
btn-sm(likely correct under the new unified icon sizing), but this is still a “pure icon” control. Given the PR goal (“square btn-icon”) and reviewer feedback that square should become the default icon-button, consider whether this should switch frombtn-circle→btn-icon(or explicitly document why it remains circular).src/app/examples/si-chat-messages/si-chat-input.html (1)
25-32: Addaria-labelfor the icon-only emoji action (example).Line 27 removes
btn-sm(fine), but this icon-only<button>relies ontitle. Please add an explicitaria-label(or bind one) so the example is accessible.Proposed change
<button type="button" class="btn btn-ghost btn-circle" + aria-label="Insert emoji" title="Insert emoji" (click)="logEvent('Emoji picker clicked')" >docs/components/layout-navigation/modals.md (1)
80-109: Docs snippet:openModal()references undefinedtemplate.Line 82 change is fine, but in the same snippet
openModal()usesthis.modalService.show(template, ...)even thoughtemplateisn’t defined (you declaredprivate modal = viewChild.required<TemplateRef<void>>('sampleModal');). Consider updating the example to pass the actual template ref.Proposed change
openModal(): void { - this.modalRef = this.modalService.show(template, { + this.modalRef = this.modalService.show(this.modal(), { /* Options go here. */ }); }projects/element-theme/src/styles/bootstrap/_buttons.scss (1)
171-189: Remove unnecessary& { }wrapper.The
& { }on lines 173-175 is redundant since there's no parent selector context needed here. The include can be placed directly in the rule block.♻️ Suggested simplification
.btn-circle, .btn-icon { - & { - @include button-icon-size($btn-icon-size-default, $btn-icon-font-size-default); - } + @include button-icon-size($btn-icon-size-default, $btn-icon-font-size-default); &.btn-lg {
🤖 Fix all issues with AI agents
In @projects/element-ng/filter-bar/si-filter-pill.component.html:
- Line 23: The button in si-filter-pill.component (the element with
class="btn-remove element-cancel btn btn-circle btn-ghost focus-inside ms-0
btn-sm") should either keep the explicit small sizing or be updated to the new
32px default: remove the btn-sm token to match the other updated icon buttons
(see datatable-grouped-rows.html and si-toast-notification.component.html)
unless a 24px size is required by design; also consider switching the shape
class from btn-circle to btn-icon if this pill should use the square icon
variant introduced in the refactor—update the class attribute accordingly and
confirm visual/design intent.
In @src/app/examples/si-dashboard/si-list-widget-css.html:
- Around line 18-23: Buttons had `btn-sm` removed; verify icon rendering and
accessibility by ensuring the pseudo-element/icon CSS still targets the button
selector (e.g., .btn-circle) and that focus/outline and hit-area spacing in
.list-group-item remain correct; if the dashboard should use the new square icon
style, replace .btn-circle with .btn-icon on these elements (check instances
matching the button snippet and the other listed ranges) and confirm via
visual/a11y checks that icon visibility, keyboard focus ring, and spacing/hit
area are preserved.
In @src/app/examples/si-icon/si-icon.html:
- Line 17: The example button in si-icon.html uses the size modifier class
"btn-sm" on the element with classes "btn btn-primary btn-circle btn-sm"; remove
the "btn-sm" token so the element uses "btn btn-primary btn-circle" to match the
standard icon-button sizing pattern used by production components and examples.
In @src/app/examples/si-wizard/si-wizard-playground.html:
- Around line 51-56: Replace the external navigation buttons' class "btn-circle"
with the new standard "btn-icon" on the icon buttons that call wizard().back()
and wizard().next() (keep "btn-circle" only if you intend to demonstrate
circular buttons); after changing the class, visually verify LTR/RTL behavior
for the "flip-rtl" class and confirm disabled styling still looks correct with
the new sizing mixins.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (14)
playwright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-no-backdrop-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--over-mode-no-backdrop-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--scroll-mode-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel--scroll-mode-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel-collapsible--open-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/si-side-panel.spec.ts-snapshots/si-side-panel--si-side-panel-collapsible--open-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--button-groups-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--button-groups-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons-element-examples-chromium-light-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--selection-buttons-element-examples-chromium-dark-linux.pngis excluded by!**/*.pngplaywright/snapshots/static.spec.ts-snapshots/buttons--selection-buttons-element-examples-chromium-light-linux.pngis excluded by!**/*.png
📒 Files selected for processing (66)
docs/components/buttons-menus/button-group.mddocs/components/layout-navigation/modals.mdplaywright/snapshots/static.spec.ts-snapshots/buttons--buttons.yamlprojects/dashboards-demo/src/app/components/widget-catalog/custom-widget-catalog.component.htmlprojects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.htmlprojects/element-ng/action-modal/si-alert-dialog/si-alert-dialog.component.htmlprojects/element-ng/action-modal/si-confirmation-dialog/si-confirmation-dialog.component.htmlprojects/element-ng/action-modal/si-delete-confirmation-dialog/si-delete-confirmation-dialog.component.htmlprojects/element-ng/action-modal/si-edit-discard-dialog/si-edit-discard-dialog.component.htmlprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.htmlprojects/element-ng/column-selection-dialog/si-column-selection-dialog.component.htmlprojects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/datepicker/si-calendar-button.component.tsprojects/element-ng/datepicker/si-date-range.component.htmlprojects/element-ng/electron-titlebar/si-electron-titlebar.component.htmlprojects/element-ng/file-uploader/si-file-uploader.component.htmlprojects/element-ng/filter-bar/si-filter-pill.component.htmlprojects/element-ng/filtered-search/si-filtered-search-value.component.htmlprojects/element-ng/filtered-search/si-filtered-search.component.htmlprojects/element-ng/main-detail-container/si-main-detail-container.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/notification-item/si-notification-item.component.htmlprojects/element-ng/number-input/si-number-input.component.htmlprojects/element-ng/password-toggle/si-password-toggle.component.htmlprojects/element-ng/photo-upload/si-photo-upload.component.htmlprojects/element-ng/pills-input/si-input-pill.component.htmlprojects/element-ng/search-bar/si-search-bar.component.htmlprojects/element-ng/side-panel/si-side-panel-content.component.htmlprojects/element-ng/side-panel/si-side-panel-content.component.scssprojects/element-ng/slider/si-slider.component.htmlprojects/element-ng/status-bar/si-status-bar.component.htmlprojects/element-ng/tabs-legacy/si-tabset/si-tabset-legacy.component.htmlprojects/element-ng/tabs/si-tab.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.htmlprojects/element-ng/tour/si-tour.component.htmlprojects/element-ng/tree-view/si-tree-view.component.htmlprojects/element-ng/wizard/si-wizard.component.htmlprojects/element-theme/src/styles/bootstrap/_button-group.scssprojects/element-theme/src/styles/bootstrap/_buttons.scsssrc/app/examples/buttons/buttoncategories.htmlsrc/app/examples/buttons/buttons.htmlsrc/app/examples/datatable/bootstrap.htmlsrc/app/examples/datatable/datatable-grouped-rows.htmlsrc/app/examples/si-chat-messages/si-attachment-list.htmlsrc/app/examples/si-chat-messages/si-chat-container.htmlsrc/app/examples/si-chat-messages/si-chat-input.htmlsrc/app/examples/si-chat-messages/si-chat-message.htmlsrc/app/examples/si-dashboard/si-list-widget-css.htmlsrc/app/examples/si-datepicker/si-modal-service.htmlsrc/app/examples/si-icon/si-icon.htmlsrc/app/examples/si-list-details/si-list-details-router.htmlsrc/app/examples/si-list-details/si-list-details.htmlsrc/app/examples/si-map/si-map-custom-cluster-popover.tssrc/app/examples/si-modals/app-table.component.htmlsrc/app/examples/si-modals/si-modal-service.htmlsrc/app/examples/si-notification-item/si-notification-item.htmlsrc/app/examples/si-wizard/si-wizard-forms.htmlsrc/app/examples/si-wizard/si-wizard-playground.html
🧰 Additional context used
🧠 Learnings (21)
📚 Learning: 2025-12-22T13:16:19.243Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: projects/element-ng/icon/element-icons.ts:166-167
Timestamp: 2025-12-22T13:16:19.243Z
Learning: In projects/element-ng/icon/element-icons.ts, the elementCheckedImageShape icon intentionally uses base64 encoding instead of utf8 encoding (which is used for other icons). This approach is considered safer and correct for this specific icon and should not be flagged as an inconsistency.
Applied to files:
projects/element-ng/status-bar/si-status-bar.component.html
📚 Learning: 2025-12-22T13:04:35.578Z
Learnt from: mistrykaran91
Repo: siemens/element PR: 1234
File: src/app/examples/ag-grid/ag-grid-empty-state.ts:20-24
Timestamp: 2025-12-22T13:04:35.578Z
Learning: The SiEmptyStateComponent in siemens/element-ng accepts icon names in kebab-case format (e.g., 'element-technical-operator') that may not be directly exported as constants in the element-icons.ts file. Do not flag these as errors if they render correctly at runtime.
Applied to files:
projects/element-ng/status-bar/si-status-bar.component.htmlprojects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/notification-item/si-notification-item.component.htmlprojects/element-ng/side-panel/si-side-panel-content.component.scssprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/tree-view/si-tree-view.component.htmlprojects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.htmlprojects/element-ng/file-uploader/si-file-uploader.component.htmlprojects/element-ng/search-bar/si-search-bar.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.htmlprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/tabs/si-tab.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/electron-titlebar/si-electron-titlebar.component.htmlprojects/element-ng/datepicker/si-calendar-button.component.tsprojects/element-ng/application-header/launchpad/si-launchpad-factory.component.htmlprojects/element-ng/tour/si-tour.component.htmlprojects/element-ng/number-input/si-number-input.component.htmlprojects/element-ng/pills-input/si-input-pill.component.htmlprojects/element-ng/tabs-legacy/si-tabset/si-tabset-legacy.component.html
📚 Learning: 2025-12-11T14:44:11.278Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:11.278Z
Learning: The si-icon component in siemens/element-ng/icon automatically applies aria-hidden="true" to its content. Do not suggest adding aria-hidden="true" to si-icon elements, as accessibility is already handled internally by the component.
Applied to files:
projects/element-ng/status-bar/si-status-bar.component.htmlprojects/element-ng/notification-item/si-notification-item.component.htmlprojects/element-ng/dashboard/widgets/si-timeline-widget/si-timeline-widget-item.component.htmlprojects/element-ng/tree-view/si-tree-view.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/threshold/si-threshold.component.htmlprojects/element-ng/tabs/si-tab.component.htmlprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/action-modal/si-alert-dialog/si-alert-dialog.component.htmlprojects/element-ng/tour/si-tour.component.htmlprojects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.htmlprojects/element-ng/pills-input/si-input-pill.component.htmlprojects/element-ng/password-toggle/si-password-toggle.component.htmlsrc/app/examples/buttons/buttons.html
📚 Learning: 2025-12-09T14:31:33.543Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:14-50
Timestamp: 2025-12-09T14:31:33.543Z
Learning: In projects/element-ng/chat-messages/si-ai-chat-container.component.html, message IDs can change, so tracking by message.id is not appropriate. The component uses `track message` (object identity) which is the correct approach for this use case.
Applied to files:
src/app/examples/si-chat-messages/si-chat-input.htmlprojects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlsrc/app/examples/si-chat-messages/si-chat-message.htmlsrc/app/examples/si-chat-messages/si-chat-container.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2025-12-17T14:01:05.705Z
Learnt from: dauriamarco
Repo: siemens/element PR: 908
File: projects/element-ng/side-panel/si-side-panel.component.ts:70-74
Timestamp: 2025-12-17T14:01:05.705Z
Learning: In the Element side-panel component (projects/element-ng/side-panel/si-side-panel.component.ts), the `xxxlMinimum` breakpoint (1920px) is intentionally hardcoded as a private static constant rather than being part of the configurable `Breakpoints` interface. This design preserves full Bootstrap compatibility (which only defines breakpoints up to xxl/1400px) while allowing Element to handle ultra-wide screens explicitly without affecting the standard breakpoint configuration.
Applied to files:
projects/element-ng/side-panel/si-side-panel-content.component.htmlprojects/element-ng/side-panel/si-side-panel-content.component.scssprojects/element-ng/navbar-vertical/si-navbar-vertical.component.htmlprojects/element-ng/electron-titlebar/si-electron-titlebar.component.htmlprojects/element-ng/datepicker/si-calendar-button.component.ts
📚 Learning: 2025-12-09T14:32:26.166Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: projects/element-ng/chat-messages/si-ai-chat-container.component.html:21-47
Timestamp: 2025-12-09T14:32:26.166Z
Learning: In Angular templates, prefer using nested *ngIf blocks over *ngSwitch for branching on message types within the chat-messages components. Use *ngIf with else branches to handle distinct cases and avoid complex switch logic in HTML templates. This guideline applies to all HTML templates under projects/element-ng/chat-messages.
Applied to files:
projects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2025-11-18T12:37:30.510Z
Learnt from: Killusions
Repo: siemens/element PR: 1040
File: projects/element-ng/chat-messages/si-chat-input.component.ts:338-349
Timestamp: 2025-11-18T12:37:30.510Z
Learning: In projects/element-ng/chat-messages/si-chat-input.component.ts, the interrupt behavior is intentionally different for button clicks vs Enter key: the button can emit interrupt even without content, but pressing Enter requires content or attachments (canSend() must be true) to emit interrupt.
Applied to files:
projects/element-ng/chat-messages/si-ai-message.component.htmlprojects/element-ng/chat-messages/si-attachment-list.component.htmlprojects/element-ng/chat-messages/si-user-message.component.htmlprojects/element-ng/chat-messages/si-chat-input.component.html
📚 Learning: 2026-01-07T20:42:06.440Z
Learnt from: dr-itz
Repo: siemens/element PR: 1262
File: projects/element-ng/date-range-filter/si-date-range-filter.component.ts:396-400
Timestamp: 2026-01-07T20:42:06.440Z
Learning: In projects/element-ng/date-range-filter/si-date-range-filter.component.ts, the `calculatedRange` values returned by the service's `resolveDateRangeFilter()` method are already normalized, guaranteeing that `end >= start`. Therefore, when calculating offsets from these normalized ranges (e.g., in `point2Changed`), Math.abs() is not needed because the result is already positive.
Applied to files:
projects/element-ng/datepicker/si-date-range.component.htmlprojects/element-ng/datepicker/components/si-calendar-direction-button.component.ts
📚 Learning: 2026-01-02T16:41:48.359Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:48.359Z
Learning: In Element docs, sizing examples may differ between documentation and implementation to illustrate multiple valid approaches. Do not flag such differences as inconsistencies; treat them as separate illustrative examples (e.g., using height/width in docs vs block-size/inline-size in implementation). Apply this understanding across Markdown documentation files to avoid false-positive review flags.
Applied to files:
docs/components/layout-navigation/modals.mddocs/components/buttons-menus/button-group.md
📚 Learning: 2026-01-07T12:19:38.754Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1240
File: projects/dashboards-ng/package.json:21-22
Timestamp: 2026-01-07T12:19:38.754Z
Learning: In the siemens/element repository, Angular peer dependencies are intentionally pinned to exact major versions (e.g., "angular/core": "21") rather than ranges in package.json files for published packages like siemens/dashboards-ng, siemens/element-ng, and siemens/element-translate-ng. This is a deliberate strategy to ensure strict version alignment.
Applied to files:
projects/element-ng/slider/si-slider.component.html
📚 Learning: 2025-12-01T09:34:50.469Z
Learnt from: Killusions
Repo: siemens/element PR: 1047
File: docs/fundamentals/ux-text-style-guide/basics.md:30-30
Timestamp: 2025-12-01T09:34:50.469Z
Learning: In docs/fundamentals/ux-text-style-guide/basics.md, the h4 heading level (####) for "Do's" and "Don'ts" within the dos-and-donts structure is intentional, even though it triggers the MD001 linting rule about heading hierarchy.
Applied to files:
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons.yaml
📚 Learning: 2025-12-16T16:19:17.950Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1207
File: playwright/snapshots/si-filtered-search.spec.ts-snapshots/si-filtered-search--si-filtered-search-playground--data-entered.yaml:38-38
Timestamp: 2025-12-16T16:19:17.950Z
Learning: Do not review files in the playwright/snapshots directory, as they contain auto-generated test artifacts. Exclude any YAML snapshot files under playwright/snapshots from code reviews in the siemens/element repository. If a YAML file is not an auto-generated snapshot, verify its origin before skipping.
Applied to files:
playwright/snapshots/static.spec.ts-snapshots/buttons--buttons.yaml
📚 Learning: 2025-12-17T04:34:55.597Z
Learnt from: chintankavathia
Repo: siemens/element PR: 1173
File: src/app/examples/si-charts/interactive/interactive.ts:5-5
Timestamp: 2025-12-17T04:34:55.597Z
Learning: Guideline: In Angular projects using OnPush change detection (zoneless mode), you only need to call ChangeDetectorRef.markForCheck() for asynchronous operations (e.g., setTimeout, setInterval, Promises, Observables) where a change is not automatically detected. Event handlers triggered by Angular template bindings (click, input, etc.) will trigger change detection automatically and do not require explicit markForCheck() calls. Apply this broadly to TS files within Angular components/services that use OnPush; use markForCheck() selectively for async work where change detection wouldn’t run otherwise.
Applied to files:
projects/element-ng/datepicker/components/si-calendar-direction-button.component.tssrc/app/examples/si-map/si-map-custom-cluster-popover.tsprojects/element-ng/datepicker/si-calendar-button.component.ts
📚 Learning: 2025-12-30T13:52:33.581Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1221
File: projects/element-ng/side-panel/si-side-panel.component.ts:136-142
Timestamp: 2025-12-30T13:52:33.581Z
Learning: Maintain the existing negative naming convention (e.g., disableBackdrop) across the siemens/element repository for consistency. Do not refactor to positive names (e.g., showBackdrop, hasBackdrop) unless the project explicitly changes the convention. In reviews, flag changes that alter established naming patterns in related files under projects/element-ng.
Applied to files:
projects/element-ng/datepicker/components/si-calendar-direction-button.component.tsprojects/element-ng/datepicker/si-calendar-button.component.ts
📚 Learning: 2025-12-09T14:33:54.441Z
Learnt from: Killusions
Repo: siemens/element PR: 998
File: src/app/examples/si-chat-messages/si-ai-message.ts:24-43
Timestamp: 2025-12-09T14:33:54.441Z
Learning: In src/app/examples/ (example/demo code) directories, allow and expect code duplication. Treat duplication as acceptable for demonstration code that aims to illustrate usage rather than enforce DRY. Do not enforce refactoring to remove duplication in example files; focus on readability and clarity of examples. This applies to all TypeScript files under src/app/examples (including nested subdirectories like si-chat-messages).
Applied to files:
src/app/examples/si-map/si-map-custom-cluster-popover.ts
📚 Learning: 2026-01-07T08:57:45.608Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1226
File: src/app/examples/si-navbar-vertical/si-navbar-vertical.ts:112-115
Timestamp: 2026-01-07T08:57:45.608Z
Learning: Do not strictly enforce badge color consistency in example files under src/app/examples. Treat badge color checks as non-blocking for TypeScript files in this directory; reviewers may still note inconsistencies, but they should not require changes for these example files. Apply this guidance broadly to all files within src/app/examples.
Applied to files:
src/app/examples/si-map/si-map-custom-cluster-popover.ts
📚 Learning: 2026-01-09T08:44:09.549Z
Learnt from: spliffone
Repo: siemens/element PR: 896
File: src/app/examples/text/text-word-break.ts:8-8
Timestamp: 2026-01-09T08:44:09.549Z
Learning: In the siemens/element repository, all example components under src/app/examples use the same selector 'app-sample' to denote live-previewer playgrounds. Treat this as a project-wide convention and do not flag such occurrences as duplicate selector issues. If reviewing files outside this directory, apply the same pattern only when the path matches src/app/examples.
Applied to files:
src/app/examples/si-map/si-map-custom-cluster-popover.ts
📚 Learning: 2025-12-11T14:44:03.830Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1185
File: projects/element-ng/navbar/si-navbar-item/si-navbar-item.component.html:11-11
Timestamp: 2025-12-11T14:44:03.830Z
Learning: When reviewing HTML that uses the si-icon component from siemens/element-ng/icon, do not add or suggest aria-hidden="true" on si-icon elements. The component automatically applies aria-hidden to its content for accessibility. If aria attributes are being added for si-icon, remove them and rely on the component's built-in handling.
Applied to files:
src/app/examples/si-icon/si-icon.html
📚 Learning: 2026-01-07T08:57:45.608Z
Learnt from: dauriamarco
Repo: siemens/element PR: 1226
File: src/app/examples/si-navbar-vertical/si-navbar-vertical.ts:112-115
Timestamp: 2026-01-07T08:57:45.608Z
Learning: Badge color consistency in example files (src/app/examples/) is not a priority and should not be strictly enforced during reviews.
Applied to files:
src/app/examples/buttons/buttons.html
📚 Learning: 2025-12-03T13:02:34.875Z
Learnt from: spike-rabbit
Repo: siemens/element PR: 1058
File: docs/fundamentals/layouts/breakpoints.md:25-31
Timestamp: 2025-12-03T13:02:34.875Z
Learning: In Element documentation for CSS breakpoints and class infixes (e.g., in docs/fundamentals/layouts/breakpoints.md), _None_ is formatted with italics to indicate the absence of a class infix (representing base mobile-first styles), while actual CSS class infixes like `sm`, `md`, `lg`, `xl`, `xxl` use inline code formatting with backticks. This semantic formatting distinction is intentional.
Applied to files:
src/app/examples/buttons/buttons.html
📚 Learning: 2026-01-02T16:41:57.274Z
Learnt from: kfenner
Repo: siemens/element PR: 1005
File: docs/fundamentals/styles/sizing.md:18-26
Timestamp: 2026-01-02T16:41:57.274Z
Learning: In Element documentation (siemens/element), sizing utility examples in docs may intentionally differ from implementation examples to demonstrate multiple valid approaches - for example, documentation might use traditional CSS properties like `height` and `width` while the implementation example uses logical properties like `block-size` and `inline-size`. These are considered different examples rather than inconsistencies.
Applied to files:
src/app/examples/buttons/buttons.html
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (56)
projects/element-ng/column-selection-dialog/si-column-selection-dialog.component.html (1)
5-5: Clarify whether this button should be updated to the new.btn-iconvariant.The cancel button removes
btn-smbut retains.btn-circle. Given the PR objective that "square buttons should be the new default icon buttons," this icon button may need to be updated from.btn-circleto.btn-icon(or replaced entirely) to align with the new button strategy.Verify that this change is intentional and that circle buttons are not being phased out as the default icon button style in this context.
projects/element-ng/chat-messages/si-attachment-list.component.html (1)
47-47: Verify alignment with the new square button default.The
btn-smmodifier was removed, but the button still usesbtn-circle. According to the PR objectives and reviewer comments, square buttons (btn-icon) should be the new default for icon buttons. Should this delete button also be migrated frombtn-circletobtn-iconto align with the new design direction?projects/element-ng/wizard/si-wizard.component.html (1)
124-124: Good consistency improvement with other circle buttons in the file.The removal of
btn-smaligns this cancel button with the other circle buttons at lines 194 and 211, which also don't use the size modifier. This standardization is consistent with the PR's unified icon sizing approach.Since this visual change increases the button size, please confirm that the layout has been tested in the horizontal wizard configuration to ensure proper spacing and alignment.
projects/element-ng/number-input/si-number-input.component.html (1)
26-45: Verifybtn-smdoesn’t regress alignment/hit-area for the number-input steppers.This swaps the stepper buttons to
btn-sm(Line 30, Line 44). Given the new icon sizing system, please sanity-check in the rendered component (enabled/disabled/readonly) that the button height +my-n3negative margins still aligns cleanly with the input and that the clickable/touch area is still acceptable. If the design intent is “square icon buttons by default,” consider whether these should be.btn-iconinstead of.btn-circle.projects/dashboards-demo/src/app/components/widget-catalog/custom-widget-catalog.component.html (1)
3-9: Consider migrating to the new defaultbtn-iconclass.According to the PR objectives, square buttons are now the default icon buttons. This close button still uses
btn-circle, which appears to be the old default. Should this button be updated to use the newbtn-iconclass instead to align with the new default styling?Additionally, verify that removing
btn-smmaintains the intended visual appearance and doesn't cause layout issues in the modal header.Based on PR objectives, square buttons should be the new default icon buttons.
projects/element-ng/main-detail-container/si-main-detail-container.component.html (1)
94-94: Verify button variant choice and size change impact.The
btn-smmodifier was removed, which will increase the button size. Additionally, this button continues to usebtn-circle(circular variant), while the PR objectives and kfenner's comment indicate that square buttons (.btn-icon) should be the new default for icon buttons.Please verify:
- Should this back button use the new
.btn-icon(square) class instead of.btn-circleto align with the new defaults?- Has the visual/layout impact of the size increase been validated in the detail header?
projects/element-ng/chat-messages/si-user-message.component.html (1)
17-26: Verify button sizing consistency across chat-message components afterbtn-smremoval.The
btn-smmodifier has been removed from both action buttons in this component (lines 19 and 31), which aligns with the PR's transition to a unified icon button sizing system. Confirm that the new default sizing works correctly for these icon buttons in all chat-message components.Likely an incorrect or invalid review comment.
projects/element-ng/column-selection-dialog/column-selection-editor/si-column-selection-editor.component.html (1)
29-29: Update line number references to lines 28 and 38.The review comment references incorrect line numbers. The class attributes being modified are on lines 28 and 38, not lines 29 and 39 (which contain dynamic class bindings). Additionally, no test or snapshot files exist for this component, so the request to verify test updates is not applicable. The actual changes—removing
btn-smto align with the new icon sizing system—are correct and preserve all button functionality.Likely an incorrect or invalid review comment.
projects/element-ng/application-header/launchpad/si-launchpad-factory.component.html (2)
91-91: AI summary is incomplete regarding the actual changes.The AI summary states that only
btn-smwas removed, but the actual changes include:
- Removed
btn-smmodifier (mentioned in summary ✓)- Removed
element-cancelclass (NOT mentioned in summary ✗)- Added positioning classes:
position-absolute top-0 end-0 mt-6 me-4(NOT mentioned in summary ✗)The positioning approach appears to have changed significantly.
89-96: Verify that the explicit positioning classes provide equivalent styling to the removedelement-cancelclass.The search found extensive usage of
element-cancelas a CSS class in other components throughout the codebase (si-modals, dialogs, filter-bar, etc.), and in test files that query for.element-cancelselectors. However, no CSS rule definition for.element-cancelwas found. This means the class is either a framework utility not visible in the searched files, or defined in compiled/built output.Without access to the actual styling rules for
element-cancel, it cannot be confirmed that the replacement with explicit positioning utilities (position-absolute top-0 end-0 mt-6 me-4) provides the same visual and functional behavior. Verify:
- The button displays correctly positioned in the top-right corner with the new utility classes
- The button size and appearance match the original styling (review mentions
btn-smremoval—confirm this doesn't negatively impact the component)- Test coverage for this component's button positioning/appearance is updated if applicable
projects/element-ng/side-panel/si-side-panel-content.component.scss (1)
116-117: The hardcoded24pxvalue is correct, but the optional refactor is not feasible.The value correctly matches
$btn-icon-size-sm: 24pxdefined inprojects/element-theme/src/styles/bootstrap/_buttons.scss. However, this variable is not exported through theall-variablesnamespace (onlybootstrap/functions,bootstrap/variables, andbootstrap/mixinsare forwarded), so the suggested refactor to useall-variables.$btn-icon-size-smwould not work. The comment "Keep in sync with $btn-icon-size-sm" is appropriate given the import constraints.projects/element-ng/chat-messages/si-chat-input.component.html (1)
51-106: No changes needed — these circular buttons are correctly styled withbtn-circle.The
btn-circleandbtn-iconvariants are intentionally different shapes (circular vs. square) and should not be interchanged. Chat interface buttons should remain circular (btn-circle), whilebtn-iconis intended for square icon buttons elsewhere in the design system. The styling is correct as-is.Likely an incorrect or invalid review comment.
src/app/examples/datatable/datatable-grouped-rows.html (1)
21-21: LGTM!Removing
btn-smaligns with the PR's goal of standardizing icon-button sizing through the new unified sizing system.projects/element-ng/toast-notification/si-toast-notification/si-toast-notification.component.html (1)
34-34: LGTM!Removing
btn-smaligns with the PR's standardization of icon-button sizing. The button will now use the default size from the unified icon-button sizing system.docs/components/buttons-menus/button-group.md (1)
91-91: The documentation is accurate. Thebtn-icon-size-smvariable in the theme implementation is defined as24px(projects/element-theme/src/styles/bootstrap/_buttons.scss, line 16), which matches the documented value on line 91.projects/element-ng/action-modal/si-alert-dialog/si-alert-dialog.component.html (1)
7-14: Verify sizing change and continued use ofbtn-circle.Two observations:
The
btn-smmodifier was removed, making this button use the default size. Verify this aligns with the intended visual design and maintains consistency with other modal close buttons across the application.The button continues to use
btn-circle. With square buttons (.btn-icon) being introduced as the new default in this PR, consider whether modal close buttons should be migrated to.btn-iconfor consistency with the new design system.projects/element-ng/status-bar/si-status-bar.component.html (1)
107-116: Verify sizing change and continued use ofbtn-circle.Two observations:
The
btn-smmodifier was removed, making this button use the default size. Verify this maintains visual consistency with the status bar layout and other icon buttons in the component.The button continues to use
btn-circle. Given that this PR introduces square buttons (.btn-icon) as the new default for icon-only buttons, consider whether this mute toggle button should be migrated to.btn-iconfor consistency with the new design system.projects/element-ng/slider/si-slider.component.html (1)
4-4: LGTM! Verify visual consistency with the new icon sizing system.The removal of
btn-smfrom both decrement and increment buttons aligns with the broader refactor to a unified icon-based sizing approach. The buttons retain theirbtn-circleandbtn-secondaryclasses, and no functional attributes have changed.Please verify that the new icon sizing system provides consistent visual sizing for these slider control buttons across all breakpoints and themes.
Also applies to: 62-62
src/app/examples/si-datepicker/si-modal-service.html (1)
9-9: LGTM! Consistent with the icon sizing refactor.The removal of
btn-smfrom the modal close button aligns with the project-wide migration to unified icon-based sizing. The button maintains all functional attributes and accessibility properties.projects/element-ng/action-modal/si-delete-confirmation-dialog/si-delete-confirmation-dialog.component.html (1)
9-9: LGTM! Sizing refactor applied consistently.The removal of
btn-smfrom the delete confirmation dialog's cancel button is consistent with the unified icon sizing approach across all modal close buttons in this PR.projects/element-ng/datepicker/components/si-calendar-direction-button.component.ts (1)
24-24: LGTM! Template sizing updated consistently.The removal of
btn-smfrom the calendar direction button template aligns with the project-wide icon sizing refactor. Component logic and public API remain unchanged.projects/element-ng/dashboard/widgets/si-list-widget/si-list-widget-item.component.html (1)
26-26: LGTM! Action button sizing updated consistently.The removal of
btn-smfrom the list widget item's action button follows the unified icon sizing refactor pattern. The custom padding (p-2) is preserved, and all functional attributes remain intact.projects/element-ng/threshold/si-threshold.component.html (2)
6-13: Verify visual appearance after removingbtn-sm.The removal of
btn-smaligns with the new unified icon sizing system introduced in this PR. However, ensure the button size remains appropriate in the threshold component's layout.
51-59: LGTM - consistent sizing change.The removal of
btn-smis consistent with the delete button above and the broader PR changes to adopt unified icon button sizing.src/app/examples/si-map/si-map-custom-cluster-popover.ts (1)
24-31: LGTM - example code updated for new sizing.The removal of
btn-smdemonstrates the new default icon button sizing in example code.src/app/examples/si-modals/app-table.component.html (1)
4-10: LGTM - modal close button sizing updated.The removal of
btn-smis consistent with the unified icon button sizing system. Theelement-cancelclass provides the close icon styling.projects/element-ng/action-modal/si-edit-discard-dialog/si-edit-discard-dialog.component.html (1)
7-14: LGTM - breaking change acknowledged.The removal of
btn-smfrom the modal close button is consistent with the PR's unified icon sizing system. This is a visual breaking change (as indicated by the PR label), so ensure it's documented in release notes.projects/element-ng/datepicker/si-calendar-button.component.ts (1)
46-57: > Likely an incorrect or invalid review comment.projects/element-ng/action-modal/si-confirmation-dialog/si-confirmation-dialog.component.html (1)
9-9: LGTM: Consistent with unified icon sizing approach.The removal of
btn-smaligns with the PR's goal of introducing icon-based sizing. The button will now use the new unified sizing system for icon buttons.src/app/examples/si-notification-item/si-notification-item.html (1)
71-71: LGTM: Consistent button sizing updates.The removal of
btn-smfrom all three quick-action buttons maintains consistency across the Custom Action Integration section and aligns with the new icon-based sizing system.Also applies to: 78-78, 113-113
projects/element-ng/side-panel/si-side-panel-content.component.html (1)
17-17: LGTM: Side panel button sizing standardized.All five side panel header buttons now use the unified icon sizing system. The removal of
btn-smis consistent across navigation, fullscreen, close, and collapse-toggle buttons.Also applies to: 38-38, 56-56, 70-70, 80-80
src/app/examples/si-modals/si-modal-service.html (1)
32-32: LGTM: Modal close button sizing standardized.All three modal templates now have consistent close button styling without
btn-sm, using the unified icon sizing approach.Also applies to: 52-52, 78-78
projects/element-ng/photo-upload/si-photo-upload.component.html (1)
64-64: LGTM: Photo upload modal button updated consistently.The removal of
btn-smfrom the modal close button aligns with the unified icon sizing approach applied across all modal components in this PR.src/app/examples/si-chat-messages/si-attachment-list.html (1)
18-24: LGTM: removingbtn-smaligns with the new icon sizing direction.Just make sure the close affordance still renders (since the button has no inner
<si-icon>) and the modal header spacing looks correct in snapshots/visual tests.src/app/examples/si-chat-messages/si-chat-message.html (1)
8-19: LGTM: consistent removal ofbtn-smon chat action buttons.Please verify the actions layout doesn’t overflow on narrow widths (especially the
actionsPosition="side"example) and snapshots are updated accordingly.Also applies to: 20-29, 40-50
projects/element-ng/electron-titlebar/si-electron-titlebar.component.html (1)
8-17: btn-sm is confirmed as the correct sizing mechanism for these buttons.
btn-xshas been removed from the codebase per the buttons refactor (commit 7a73280), andbtn-smnow standardizes to 24px across all button types. The titlebar component was updated in that refactor and correctly usesbtn-sm.However, the inconsistency remains valid:
electron-title-bar-btnis applied only to the menu button (line 31) but not to the back/forward buttons (lines 10, 20). Since the component styles don't define any rules for this class, clarify whether it should be applied consistently to all three buttons for uniform spacing, or whether the menu button requires this distinction for a specific reason.projects/element-ng/tour/si-tour.component.html (1)
32-32: LGTM! Sizing change aligns with icon button unification.The removal of
btn-smis consistent with the PR's unified icon button sizing approach.projects/element-ng/tree-view/si-tree-view.component.html (1)
4-4: LGTM! Sizing changes align with icon button unification.The removal of
btn-smfrom both expand/collapse buttons is consistent with the PR's unified icon button sizing approach.Also applies to: 12-12
src/app/examples/datatable/bootstrap.html (1)
41-41: LGTM! Consistent sizing changes across all dropdown buttons.The removal of
btn-smis consistently applied to all four dropdown buttons, aligning with the PR's unified icon button sizing approach.Also applies to: 75-75, 104-104, 133-133
projects/dashboards-ng/src/components/dashboard-toolbar/si-dashboard-toolbar.component.html (1)
11-11: LGTM! Sizing change aligns with icon button unification.The removal of
btn-smfrom the icon-only edit button (false branch) is consistent with the PR's unified icon button sizing approach. The label-visible variant remains unchanged.src/app/examples/si-chat-messages/si-chat-container.html (1)
31-31: LGTM! Sizing changes align with icon button unification.The removal of
btn-smfrom the per-action button and modal close button is consistent with the PR's unified icon button sizing approach.Also applies to: 80-80
projects/element-ng/chat-messages/si-ai-message.component.html (1)
16-16: LGTM! Button sizing updated consistently.Removal of
btn-smaligns with the PR's unified icon-based sizing system. The buttons will now use the default icon button size.Also applies to: 28-28
projects/element-ng/filtered-search/si-filtered-search-value.component.html (1)
122-132: LGTM! Button sizing updated consistently.The change from
btn-xstobtn-smaligns with the PR's unified icon-based sizing system. The clear button maintains proper accessibility witharia-labeland disabled state handling.projects/element-ng/search-bar/si-search-bar.component.html (1)
22-32: LGTM! Button sizing updated consistently.The change from
btn-xstobtn-smaligns with the PR's unified icon-based sizing system. The cancel button maintains proper accessibility witharia-labeland handles disabled/readonly states correctly.projects/element-ng/password-toggle/si-password-toggle.component.html (1)
3-10: Size change looks fine; just sanity-check field alignment.Line 5 moves
btn-xs→btn-sm. This is consistent with the sizing standardization, but please verify it doesn’t misalign/overflow in the smallest input variants (especially in dense forms).projects/element-ng/tabs-legacy/si-tabset/si-tabset-legacy.component.html (1)
62-68: Event propagation is properly stopped; no functional issue with this change.The
closeTab($event, tab)method already callsevent.stopPropagation()as the first operation, preventing the click from triggering the parent tab'sselectTab()handler. Additionally, the btn-xs → btn-sm class change is not a size increase—both maintain 24px for icon buttons per the button refactor (commit 7a73280). No truncation risk or functional concern.src/app/examples/buttons/buttoncategories.html (1)
10-36: LGTM!The button class updates align well with the new unified icon sizing system. The navigation buttons appropriately use
btn-lgfor prominence, while the action buttons now use the default size. All buttons retain properaria-labelattributes for accessibility.src/app/examples/si-list-details/si-list-details.html (1)
21-44: LGTM!Removing
btn-smfrom these circle buttons aligns with the unified icon sizing system. The buttons will now use the default size ($btn-icon-size-default), and accessibility attributes are properly maintained.src/app/examples/si-list-details/si-list-details-router.html (1)
21-44: LGTM!Changes are consistent with the sibling file
si-list-details.html. Thebtn-smremoval aligns with the unified icon sizing system introduced in this PR.projects/element-ng/file-uploader/si-file-uploader.component.html (1)
56-90: LGTM!The
btn-smremoval from these action buttons aligns with the unified icon sizing system. All buttons maintain proper accessibility attributes viatitleandaria-labelbindings. Note that this is a component file, so the size change will affect all instances of the file uploader across the application.src/app/examples/buttons/buttons.html (3)
47-127: Well-organized section for the new square icon buttons.The new "Icon buttons" section properly demonstrates the
.btn-iconclass with all color variants. All buttons correctly includearia-labelattributes. The section placement above circle buttons aligns with the reviewer's feedback that square buttons should be the new default.
231-260: Clarify intent of combining.btn-iconand.btn-circleclasses.These buttons use both
.btn-iconand.btn-circletogether. Based on the SCSS,.btn-circlewill override the border-radius to 50% since it's defined after.btn-icon. If this is intentional to demonstrate that.btn-iconsizing applies to circle variants, consider adding a comment or adjusting thearia-labelto clarify (e.g., "large circle button" instead of duplicating "default circle button").
283-295: LGTM!The disabled state toggle is well-implemented using a form switch with proper label association via
for/idattributes.projects/element-theme/src/styles/bootstrap/_buttons.scss (3)
11-16: Verify intentional same font-size for default and small variants.
$btn-icon-font-size-sm(24px) equals$btn-icon-font-size-default(24px), while the button sizes differ (24px vs 32px). This keeps the icon visually consistent while the button shrinks—confirm this is the intended behavior.
152-161: LGTM on the unified selector approach.The
:is(.btn-circle, .btn-icon, .btn-close):not(.btn-group .btn)selector cleanly consolidates shared rules while properly excluding button-group contexts where different sizing applies. The.iconmargin reset is appropriately scoped here.
191-198: Verify.btn-close:not(.btn-tertiary)selector intent.This selector applies ghost styling to
.btn-closeunless it has.btn-tertiary. Confirm this is intentional—it means.btn-close.btn-tertiarywill not receive the ghost background/color variables defined here and will instead inherit from the.btn-tertiaryrules above.
dauriamarco
force-pushed
the
feat/btn-icon
branch
from
7a73280 to
bf06d66
Compare
dauriamarco
removed
the
breaking-changes
panch1739
left a comment
panch1739
left a comment
There was a problem hiding this comment.
@dauriamarco Perfect! I just have one observation.
The size of the icons buttons is 40px (large). I think it would be nicer if we show all the circle/square buttons in the default 32px, and just show the 40px and 24px variants in the sizing section.
spike-rabbit
left a comment
spike-rabbit
left a comment
There was a problem hiding this comment.
Looks good, would just be good if you could get rid of the :nots
dauriamarco
force-pushed
the
feat/btn-icon
branch
from
bf06d66 to
1e3912b
Compare
Sounds good 👍 If that works for you, I’ll make this change in one of the other two PRs (#1263 or #1329) where the sizing will be correct. |
dauriamarco
force-pushed
the
feat/btn-icon
branch
from
1e3912b to
493ee29
Compare
spike-rabbit
left a comment
spike-rabbit
left a comment
There was a problem hiding this comment.
The code is perfect now. I am just not 100% about the NOTE.
@panch1739 this is the current note which will show also in the changelog.
NOTE: The new square icon buttons (`.btn-icon`) are introduced as the
new default style for icon buttons. The existing circle buttons (`.btn-circle`)
are now reserved for cases where a circular shape is explicitly required.
I think we should also introduce some guidance for projects here. Should they change now stuff?
@spike-rabbit Sounds perfect, I also have a docs PR #1218 updating that info and switching relevant images. I think that we would need to prepare some documentation or tutorial, because im sure there will be questions regardless the note in the code... The note is fine as is written, but if you want more detail, maybe we can say something like: "Square buttons are now the default. Circular buttons remain available but should only be used for floating or standalone actions, or when the surrounding container explicitly requires a circular shape. What do you think? |
Introduces square icon buttons (`.btn-icon`) in addition to the existing circle buttons (`.btn-circle`). They follow the same sizing options (lg, sm, xs) and integrate seamlessly with button groups. NOTE: The new square icon buttons (`.btn-icon`) are introduced as the new default style for icon buttons. The existing circle buttons (`.btn-circle`) are now reserved for cases where a circular shape is explicitly required.
spike-rabbit
force-pushed
the
feat/btn-icon
branch
from
493ee29 to
b5c2fe2
Compare
|
|
Introduce the square button variant.
feat(buttons): add square btn-icon
Introduces a new square .btn-icon variant and refactors icon-button styling to unify .btn-circle, .btn-icon, and .btn-close handling.
Key changes
Other notes