fix(action): maintain equal height when text is not enabled in a small scale #9051

driskull · 2024-04-03T16:41:50Z

Related Issue: #8900

Summary

Maintain equal height when text is not enabled in a small scale
Add screenshot test

…l scale. #8900

driskull · 2024-04-03T18:05:24Z

@ashetland this will need a design review. Changing the height without text seems to have some visual changes happening in other stories.

jcfranco

Noice!

jcfranco · 2024-04-04T15:03:45Z

packages/calcite-components/src/components/action/action.tsx

-    const ariaLabel = `${label || text}${indicator ? ` (${messages.indicator})` : ""}`;
+    const labelFallback = label || text;
+    const ariaLabel = labelFallback
+      ? `${labelFallback}${indicator ? ` (${messages.indicator})` : ""}`


This should use a localized template. Since this is already in production, can you create an issue? We can link to action’s doc site page for the repro case since this is a screen reader label.

created #9071

jcfranco · 2024-04-04T15:05:19Z

packages/calcite-components/src/components/action/action.stories.ts

@@ -210,6 +210,20 @@ export const indicatorNoTextEnabledNoIcon_TestOnly = (): string => html`
  <calcite-action indicator active text="click-me"></calcite-action>
 `;

+export const noTextHeight_TestOnly = (): string =>
+  html`<h2>All actions should be the same height</h2>


macandcheese · 2024-04-04T19:00:52Z

I do think this will need approval from @ashetland / @SkyeSeitz as there could be significant changes to an app's UI depending on set up here. Some places that could be lessened by swapping action scale / etc., but that will require a lot of spot checking.

ashetland · 2024-04-08T18:39:47Z

Is this another "bug" where the fix might be seen as breaking? Seems like something we should correct, but maybe it should wait? cc @jcfranco

driskull · 2024-04-08T21:17:25Z

Are the height differences also in Figma? I would consider this a fix if not.

SkyeSeitz · 2024-04-08T21:32:30Z

The height differences are not present in Figma. The height of the Action remains the same regardless if text is enabled.

ashetland · 2024-04-08T22:03:59Z

If we're okay with moving forward with this I can review the screenshots and add the visual changes label to the issue.

driskull · 2024-04-08T22:43:55Z

Lets see what @jcfranco thinks. I think we can do it if we consider it a bug.

jcfranco · 2024-04-09T14:13:38Z

Sorry for the late reply. We should treat this one as a bug/fix since the design spec has no height differences.

ashetland · 2024-04-09T15:12:00Z

Sounds good. I added the visual changes label to the issue.

driskull · 2024-04-09T15:59:42Z

@ashetland can you review and accept the screenshot tests?

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-design-tokens: 2.2.0</summary> ## [2.2.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-design-tokens@2.1.2...@esri/calcite-design-tokens@2.2.0) (2024-04-30) ### Features * Ensure all components inherit font-family ([#8388](#8388)) ([90f8923](90f8923)) </details> <details><summary>@esri/eslint-plugin-calcite-components: 1.2.0</summary> ## [1.2.0](https://github.com/Esri/calcite-design-system/compare/@esri/eslint-plugin-calcite-components@1.1.0...@esri/eslint-plugin-calcite-components@1.2.0) (2024-04-30) ### Features * **enforce-ref-last-prop:** Add fixer ([#8671](#8671)) ([688bc51](688bc51)) ### Bug Fixes * **slider:** Improve snapping + step logic ([#8169](#8169)) ([8b83042](8b83042)) </details> <details><summary>@esri/calcite-components: 2.8.0</summary> ## [2.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@2.7.1...@esri/calcite-components@2.8.0) (2024-04-30) ### Features * Add calciteInvalid event for form validation ([#9211](#9211)) ([a504508](a504508)) * Add validity property to form components ([#9210](#9210)) ([e49902b](e49902b)) * **carousel:** Add Carousel and Carousel Item components ([#8995](#8995)) ([b1ba428](b1ba428)) * **enforce-ref-last-prop:** Add fixer ([#8671](#8671)) ([688bc51](688bc51)) * Ensure all components inherit font-family ([#8388](#8388)) ([90f8923](90f8923)) * **input-time-zone:** Add readonly support ([#9111](#9111)) ([153a122](153a122)) * **input-time-zone:** Allow clearing value ([#9168](#9168)) ([193bb7d](193bb7d)) * **list-item-group:** Update list item group styles ([#9072](#9072)) ([c734849](c734849)) * **navigation-logo:** Add heading level ([#9054](#9054)) ([c2beba8](c2beba8)) * **pagination:** Add navigation methods ([#9154](#9154)) ([5ca6a5f](5ca6a5f)) * **slider:** Add support for custom label formatting ([#9179](#9179)) ([710d1ee](710d1ee)) * **slider:** Allow configuring fill behavior ([#9170](#9170)) ([1b2cdbf](1b2cdbf)) * **tile, tile-group:** Support single, multi, single-persist, none selection modes and icon and border selection appearances ([#9159](#9159)) ([2d77470](2d77470)) ### Bug Fixes * **action:** Maintain equal height when text is not enabled in a small scale ([#9051](#9051)) ([407824a](407824a)) * **chip-group:** Improve programmatic Chip selection behavior ([#9213](#9213)) ([b7a4c77](b7a4c77)) * **combobox:** Update the focus and hover chevron icon color ([#9187](#9187)) ([a1317da](a1317da)) * **combobox, input-time-zone:** Improve readOnly behavior and styling ([#9222](#9222)) ([606d80f](606d80f)) * **combobox:** Fix aria-role for proper voiceover support ([#9039](#9039)) ([eebe8ca](eebe8ca)) * **combobox:** Update the focused chevron icon color ([#9202](#9202)) ([27ac02d](27ac02d)) * **date-picker:** Ensure `proximitySelectionDisabled` resets range on post-range-commit selection ([#9171](#9171)) ([f466b14](f466b14)) * **date-picker:** Restore focus on date when navigating month with arrow/page keys ([#9063](#9063)) ([db109e0](db109e0)) * Fix memory leak affecting components using conditionally-rendered slots ([#9208](#9208)) ([28701b6](28701b6)) * **input-date-picker:** Update the focus and hover chevron icon color ([#9146](#9146)) ([ca895e9](ca895e9)) * **input-time-zone:** Ensure selected item is properly displayed when there are other items with the same offset ([#9134](#9134)) ([1f94903](1f94903)) * **input, input-number, input-text, input-date-picker, input-time-picker, filter, menu-item:** Ignore canceled events and enforce cancellations where needed ([#8952](#8952)) ([d0fa977](d0fa977)) * **list, list-item, list-item-group:** Improve drag experience by indenting items ([#9169](#9169)) ([88aab49](88aab49)) * **list, list-item:** Support keyboard sorting in screen readers ([#9038](#9038)) ([b2e1b9b](b2e1b9b)) * **list:** Closed list-items no longer render extra space ([#9031](#9031)) ([3985004](3985004)) * **panel:** Support cancelling the esc key event to prevent closing a panel ([#9032](#9032)) ([ecfa5f1](ecfa5f1)) * **preset:** Update the focus outline color ([#9098](#9098)) ([725f47c](725f47c)) * **radio-button:** Display validation message in parent group ([#9218](#9218)) ([b1e869f](b1e869f)) * **select:** Update the focus and hover chevron icon color and select focus and hover background color ([#9160](#9160)) ([b187340](b187340)) * **slider:** Fix fill placement and tick styling when ranged ([#9204](#9204)) ([a84d831](a84d831)) * **slider:** Improve snapping + step logic ([#8169](#8169)) ([8b83042](8b83042)) * **slider:** Rename `highlightPlacement` to `fillPlacement` ([#9195](#9195)) ([9e5a713](9e5a713)) * **slider:** Style ticks according to the fill placement ([#9196](#9196)) ([5eb4e10](5eb4e10)) * **stepper-item:** Remove delay in highlighting item ([#8996](#8996)) ([bcf23dd](bcf23dd)) * **tree-item:** Do not select when chevron clicked ([#9115](#9115)) ([99ad8f1](99ad8f1)) ### Dependencies * The following workspace dependencies were updated * devDependencies * @esri/calcite-design-tokens bumped from ^2.2.0-next.0 to ^2.2.0 * @esri/eslint-plugin-calcite-components bumped from ^1.2.0-next.1 to ^1.2.0 </details> <details><summary>@esri/calcite-components-angular: 2.8.0</summary> ## [2.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-angular@2.7.1...@esri/calcite-components-angular@2.8.0) (2024-04-30) ### Miscellaneous Chores * **@esri/calcite-components-angular:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from ^2.8.0-next.20 to ^2.8.0 </details> <details><summary>@esri/calcite-components-react: 2.8.0</summary> ## [2.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@2.7.1...@esri/calcite-components-react@2.8.0) (2024-04-30) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from ^2.8.0-next.20 to ^2.8.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

fix(action): maintain equal height when text is not enabled in a smal…

03211d0

…l scale. #8900

github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Apr 3, 2024

github-actions bot assigned driskull Apr 3, 2024

driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Apr 3, 2024

fix test

881ff2c

driskull added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Apr 3, 2024

driskull marked this pull request as ready for review April 3, 2024 17:26

driskull requested a review from a team as a code owner April 3, 2024 17:26

driskull requested review from ashetland and macandcheese April 3, 2024 22:44

jcfranco approved these changes Apr 4, 2024

View reviewed changes

driskull mentioned this pull request Apr 5, 2024

Action: Create localized template for aria-label #9071

Open

6 tasks

driskull merged commit 407824a into main Apr 9, 2024
16 checks passed

driskull deleted the dris0000/action-equal-heights-8900 branch April 9, 2024 18:30

github-actions bot added this to the 2024-04-30 - Apr Release milestone Apr 9, 2024

calcite-admin mentioned this pull request Apr 9, 2024

chore: release main #9050

Merged

calcite-admin mentioned this pull request May 14, 2024

chore: release main #9237

Merged

calcite-admin mentioned this pull request May 28, 2024

chore: release hotfix #9438

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(action): maintain equal height when text is not enabled in a small scale #9051

fix(action): maintain equal height when text is not enabled in a small scale #9051

driskull commented Apr 3, 2024

driskull commented Apr 3, 2024

jcfranco left a comment

jcfranco Apr 4, 2024

driskull Apr 5, 2024

jcfranco Apr 4, 2024

macandcheese commented Apr 4, 2024

ashetland commented Apr 8, 2024

driskull commented Apr 8, 2024

SkyeSeitz commented Apr 8, 2024

ashetland commented Apr 8, 2024

driskull commented Apr 8, 2024

jcfranco commented Apr 9, 2024

ashetland commented Apr 9, 2024

driskull commented Apr 9, 2024

fix(action): maintain equal height when text is not enabled in a small scale #9051

fix(action): maintain equal height when text is not enabled in a small scale #9051

Conversation

driskull commented Apr 3, 2024

Summary

driskull commented Apr 3, 2024

jcfranco left a comment

Choose a reason for hiding this comment

jcfranco Apr 4, 2024

Choose a reason for hiding this comment

driskull Apr 5, 2024

Choose a reason for hiding this comment

jcfranco Apr 4, 2024

Choose a reason for hiding this comment

macandcheese commented Apr 4, 2024

ashetland commented Apr 8, 2024

driskull commented Apr 8, 2024

SkyeSeitz commented Apr 8, 2024

ashetland commented Apr 8, 2024

driskull commented Apr 8, 2024

jcfranco commented Apr 9, 2024

ashetland commented Apr 9, 2024

driskull commented Apr 9, 2024