feat(dropdown-item): update spacing of icons #9330
Conversation
github-actions
bot
added
the
enhancement
jcfranco
added
the
pr ready for visual snapshots
jcfranco
changed the title
anveshmekala
added
pr ready for visual snapshots
anveshmekala
added
pr ready for visual snapshots
anveshmekala
added
pr ready for visual snapshots
anveshmekala
added
pr ready for visual snapshots
| @@ -1,9 +1,9 @@ | |||
| export const CSS = { | |||
| container: "container", | |||
| containerNone: "container--none-selection", | |||
There was a problem hiding this comment.
SideNote: Sorting is handled in a followup refactor PR.
anveshmekala
added
pr ready for visual snapshots
|
Lookin' gewd! |
packages/calcite-components/src/components/dropdown-item/dropdown-item.scss
Outdated
Show resolved
Hide resolved
packages/calcite-components/src/components/dropdown-item/dropdown-item.scss
Outdated
Show resolved
Hide resolved
| @@ -125,7 +125,7 @@ export class DropdownItem implements InteractiveComponent, LoadableComponent { | |||
| * | |||
| * @internal | |||
| */ | |||
| @Prop() scale: Scale = "m"; | |||
| @Prop({ reflect: true }) scale: Scale = "m"; | |||
There was a problem hiding this comment.
Per our conversation, let's discuss reflection of internal properties as a pattern, as it simplifies styling without significant trade-offs.
There was a problem hiding this comment.
Reflecting @internal props for styling purposes will help us improves the maintainability and readability of styles. Though it's internal since they aren't document for public use it is safe to reflect and take advantage of the attribute selector in CSS instead of creating dynamic classes.
Considering this PR as example, reflecting prop helped us avoid cumbersome styling code and there are other instance in the repo where internal props are reflected. We can use this as a pattern if the reflection helps improving the code without affecting UI/UX.
There was a problem hiding this comment.
Yeah its a trade off discussion...
Is showing an undocumented attribute terrible? No.
Is not showing an undocumented attribute better? Probably.
So its really about if its worth the work to use an internal shadow DOM class vs relying on an attribute.
There was a problem hiding this comment.
So its really about if its worth the work to use an internal shadow DOM class vs relying on an attribute.
Agreed. From what I've seen, the extra work of adding root container classes to mimic host and reflected internal attributes (e.g., scale-related styling) can feel like overkill. Since we usually avoid reflecting props on the host element, leveraging the reflection of internal properties seems simpler and cleaner in this case.
**Related Issue:** # N/A ## Summary Removes unused CSS classes. _Note_: Suggested to install after #9330
…otfix-release * origin/main: chore: release next feat(table): Add `selection-display` property (#9355) refactor(dropdown-item): remove unused css classes (#9349) chore: release next feat(dropdown-item): update spacing of icons (#9330) chore: release next feat(flow-item): add content-top slot (#9344) chore: release next feat(flow-item): add content-bottom slot (#9346) chore: release next fix(input, input-number, input-text): ensure autofocus is available on HTMLElement (#9343) (#9347)
Related Issue: #7095
Summary
Updates spacing of icons in
calcite-dropdown-item.