feat(alert): make component responsive #7755

jcfranco · 2023-09-16T09:23:22Z

Related Issue: #6673

Summary

Updates alert to be responsive per the design spec.

Additional changes

simplified class names (alert prefix is no longer necessary)
simplified styles by fixing issues reported by no-descending-specificity stylelint rule
requestedIcon is now computed at render time to simplify component
refactored class names (e.g., used BEM-like convention to state/modifier classes) and applied them according to conventions
clean up test story

driskull

👍 💯 🥇

driskull · 2023-09-18T22:31:34Z

packages/calcite-components/src/components/alert/alert.scss

+  container: responsive-container / inline-size;
+  border-radius: var(--calcite-border-radius);
+  border-block-start: 0 solid transparent;
+  border-inline: 1px solid var(--calcite-ui-border-3);


I wonder if these border px values should come from a token?

I stored them in a sass variable for now to DRY things up w/o adding a CSS prop.

driskull · 2023-09-18T22:32:01Z

packages/calcite-components/src/components/alert/alert.scss

+  border-inline: 1px solid var(--calcite-ui-border-3);
+  border-block-end: 1px solid var(--calcite-ui-border-3);
+  inline-size: var(--calcite-alert-width);
+  max-inline-size: calc(100% - (var(--calcite-alert-edge-distance) * 2 + 2px));


What is the 2px here from? can that come from a token?

Agreed on using internal CSS props or Sass vars for these. @macandcheese could you help us out with this?

Hm, this is an older one, maybe to compensate for the 1px border on either side? May not be necessary anymore.

Doesn't seem to be necessary. Removed!

driskull · 2023-09-18T22:32:41Z

packages/calcite-components/src/components/alert/alert.scss

+  w-full
+  overflow-hidden;
+  inset-inline: 0;
+  inset-block-start: -2px;


For these 2px values maybe a sass variable or token would be nice

Maybe we can do this in a follow-up PR. I'm not entirely sure if all 2px values represent the same thing. 🤔

Yep sounds good 👍

driskull · 2023-09-18T22:34:50Z

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

+        <calcite-icon
+          flipRtl={this.iconFlipRtl}
+          icon={icon}
+          scale={this.scale === "l" ? "m" : "s"}


Maybe a function for setting this scale value? This logic appears twice.

I like the idea of having a util for internal icon scale adjustment. Especially, since it's sprinkled across many components. These seem to be the only exceptions I found:

calcite-design-system/packages/calcite-components/src/components/popover/popover.tsx

Line 545 in 8a575e4

<calcite-icon icon="x" scale={this.scale === "l" ? "m" : this.scale} />

calcite-design-system/packages/calcite-components/src/components/modal/modal.tsx

Line 300 in 796bf90

this.scale === "s" ? "s" : this.scale === "m" ? "m" : this.scale === "l" ? "l" : null

@ashetland @SkyeSeitz All internal icons scales should be following this (m icon for scale="l" and s icon for scale="s" + scale="m") , right?

Yep, that's correct.

jcfranco · 2023-09-19T20:41:34Z

@macandcheese @ashetland @SkyeSeitz Could you review the visual changes? 🙇

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 1.9.0</summary> ## [1.9.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2023-10-03) ### Features * **action-group:** Add css custom properties to define gap and padding when layout is "grid" ([#7763](#7763)) ([b9bd0de](b9bd0de)) * **action-menu:** Add appearance property to configure trigger appearance ([#7867](#7867)) ([36ceaf1](36ceaf1)) * **alert:** Make component responsive ([#7755](#7755)) ([66222b5](66222b5)) * **block, input-time-picker, notice:** Adds `open`, `close`, `beforeOpen`, and `beforeClose` events for consistent event handling ([#7494](#7494)) ([04ce758](04ce758)) * **checkbox:** Add WCAG AA recommended minimum 24px square hotspot optimized for touch users ([#7773](#7773)) ([f13e2c4](f13e2c4)) * **flow, flow-item:** Allow calciteFlowItemBack event to be cancelled ([#7855](#7855)) ([b74b4df](b74b4df)) * **input-date-picker, input-time-picker:** Add status property ([#7915](#7915)) ([4d53346](4d53346)) * **input-time-zone:** Add max-items support ([#7705](#7705)) ([c698c27](c698c27)) * **input-time-zone:** Add time zone offset and name mode ([#7913](#7913)) ([80bd6ae](80bd6ae)) * **list:** Add newIndex and oldIndex event details to calciteListOrderChange event ([#7874](#7874)) ([0d5cc20](0d5cc20)) * **pagination:** Enable responsive layout ([#7722](#7722)) ([933a910](933a910)) * **panel, flow-item:** Add support for collapsing content ([#7857](#7857)) ([855754d](855754d)) ### Bug Fixes * **action-bar:** Improve overflowing horizontal actions. ([#7877](#7877)) ([52f2d2a](52f2d2a)) * **action-bar:** Overflow actions when overflowActionsDisabled is set to false ([#7873](#7873)) ([3dcceb0](3dcceb0)) * **action-menu:** Update active selection to not use the action's active property ([#7911](#7911)) ([50f85f1](50f85f1)) * **alert:** Regression fix to restore `calciteAlertBeforeOpen` and `calciteAlertOpen` event emitting ([#7767](#7767)) ([6bbae35](6bbae35)) * **button:** Provides context for AT users when used as reference element for collapsible content ([#7658](#7658)) ([50cb3a6](50cb3a6)) * **combobox:** Clears input value on blur ([#7721](#7721)) ([e04cc4e](e04cc4e)) * **combobox:** Fix filtering to avoid showing unrelated items ([#7704](#7704)) ([b6d32f3](b6d32f3)) * **dropdown-group:** Set selectionMode on slotted dropdown-item children ([#7897](#7897)) ([aa0731a](aa0731a)) * **dropdown:** Support dispatching enter key event on dropdown trigger ([#7752](#7752)) ([ba92463](ba92463)) * **select:** Allow setting an option value to an empty string ([#7769](#7769)) ([adca6ec](adca6ec)) * **stepper:** Improves AT Users experience with screen readers ([#7691](#7691)) ([071dec7](071dec7)) * **tab-nav:** Update indicator position when tab icon changes ([#7768](#7768)) ([cb877b3](cb877b3)) * **table:** Fix selection display in RTL ([#7907](#7907)) ([b4c8508](b4c8508)) * **tree:** Avoid modifying selected items for "none" selection-mode ([#7904](#7904)) ([0bd4a12](0bd4a12)) * **tree:** Fix tree keyboard selection issue ([#7908](#7908)) ([53d6c12](53d6c12)) * **tree:** Update tree selection per design spec ([#7852](#7852)) ([06b3594](06b3594)) </details> <details><summary>@esri/calcite-components-react: 1.9.0</summary> ## [1.9.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected]) (2023-10-03) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize undefined versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from ^1.9.0-next.18 to ^1.9.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>

@alhridoy

…7973) # Adopted #7935 cc @alhridoy --- **Related Issue:** #7765 ## Summary This pull request extracts the logic for adjusting internal icon scales into a separate utility function. This function is now used across all components, making the code DRYer and easier to maintain. ## Changes - Created a new utility function `adjustIconScale` in `iconScaleAdjuster.ts`. - Replaced inline logic for adjusting icon scales with calls to `adjustIconScale` in all relevant components. ## Testing - All unit tests pass. - Manual testing in the browser confirms that icons still display correctly at all scales. This change stems from issue #7755 and is expected to make the codebase easier to maintain by reducing repetition of this logic. --------- Co-authored-by: Al-Iqram Elahee <[email protected]>

Roll back alert responsiveness (#7755) in favor of upcoming alert queue UX (#2835)

**Related Issue:** N/A ## Summary Rolls back responsiveness (#7755) in favor of upcoming alert stack UX (#2835). See #7921 (comment) for more info.

**Related Issue:** N/A Rolls back responsiveness (#7755) in favor of upcoming alert stack UX (#2835). See #7921 (comment) for more info.

feat(alert): make component responsive

e457c12

jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Sep 16, 2023

github-actions bot assigned jcfranco Sep 16, 2023

github-actions bot added the enhancement Issues tied to a new feature or request. label Sep 16, 2023

update doc

c41756f

jcfranco 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 Sep 16, 2023

add missing spacing util

e7d12d5

jcfranco 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 Sep 16, 2023

jcfranco added 2 commits September 18, 2023 12:45

merge main

d88e10d

Merge branch 'main' into jcfranco/6773-responsivify-alert

6f178ce

jcfranco 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 Sep 18, 2023

jcfranco added 2 commits September 18, 2023 14:39

tidy up

1b2be0b

fix tests

d05c794

jcfranco 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 Sep 18, 2023

jcfranco marked this pull request as ready for review September 18, 2023 22:23

jcfranco requested a review from a team as a code owner September 18, 2023 22:23

driskull approved these changes Sep 18, 2023

View reviewed changes

jcfranco mentioned this pull request Sep 18, 2023

Create reusable util for adjusting internal icon scale used within a component #7765

Closed

2 tasks

jcfranco added 4 commits September 20, 2023 11:02

merge main

a41107c

update footer border spacing

d3ad965

add delay to stories

10c2a2b

fix issue caused by reusing the same named slot

9e2a062

jcfranco 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 Sep 22, 2023

fine-tune screenshot delays

4c7fff3

jcfranco added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Sep 22, 2023

jcfranco added 2 commits September 26, 2023 12:18

tweak close button across breakpoints

2a7dbd5

tidy up

5aa70ca

jcfranco 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 Sep 26, 2023

jcfranco added 4 commits September 27, 2023 15:17

fix incorrect variable used for block spacing

e7d3416

tidy up

34125fc

tidy up

6518808

tidy up

cb60ed5

jcfranco 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 Sep 27, 2023

jcfranco added 2 commits September 28, 2023 14:53

prevent focus outline from being clipped

5cfbed6

center close button icon from s breakpoint onward

ef7048a

jcfranco 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 Sep 28, 2023

jcfranco merged commit 66222b5 into main Sep 29, 2023
14 checks passed

jcfranco deleted the jcfranco/6773-responsivify-alert branch September 29, 2023 17:30

github-actions bot added this to the 2023 September Priorities - Main release milestone Sep 29, 2023

calcite-admin mentioned this pull request Sep 29, 2023

chore: release main #7718

Merged

alhridoy mentioned this pull request Oct 4, 2023

refactor: extract icon scale adjustment logic into utility function #7935

Closed

jcfranco mentioned this pull request Oct 11, 2023

refactor: extract icon scale adjustment logic into utility function #7973

Merged

jcfranco added a commit that referenced this pull request Nov 17, 2023

revert: feat(alert): make component responsive

c3c72df

Roll back alert responsiveness (#7755) in favor of upcoming alert queue UX (#2835)

jcfranco added a commit that referenced this pull request Nov 17, 2023

revert: feat(alert): make component responsive

52bb096

Roll back alert responsiveness (#7755) in favor of upcoming alert queue UX (#2835)

jcfranco mentioned this pull request Nov 17, 2023

revert: feat(alert): make component responsive #8195

Merged

jcfranco added a commit that referenced this pull request Nov 21, 2023

revert: feat(alert): make component responsive (#8195)

2f4ff6c

**Related Issue:** N/A ## Summary Rolls back responsiveness (#7755) in favor of upcoming alert stack UX (#2835). See #7921 (comment) for more info.

benelan pushed a commit that referenced this pull request Nov 24, 2023

revert: feat(alert): make component responsive (#8195)

19cf05f

**Related Issue:** N/A Rolls back responsiveness (#7755) in favor of upcoming alert stack UX (#2835). See #7921 (comment) for more info.

benelan pushed a commit that referenced this pull request Nov 24, 2023

revert: feat(alert): make component responsive (#8195)

330eb44

**Related Issue:** N/A Rolls back responsiveness (#7755) in favor of upcoming alert stack UX (#2835). See #7921 (comment) for more info.

benelan pushed a commit that referenced this pull request Nov 26, 2023

revert: feat(alert): make component responsive (#8195)

bb017ed

**Related Issue:** N/A Rolls back responsiveness (#7755) in favor of upcoming alert stack UX (#2835). See #7921 (comment) for more info.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(alert): make component responsive #7755

feat(alert): make component responsive #7755

jcfranco commented Sep 16, 2023 •

edited

Loading

driskull left a comment

driskull Sep 18, 2023

jcfranco Sep 27, 2023

driskull Sep 18, 2023

jcfranco Sep 18, 2023

macandcheese Sep 19, 2023

jcfranco Sep 27, 2023

driskull Sep 18, 2023

jcfranco Sep 27, 2023

driskull Sep 27, 2023

driskull Sep 18, 2023

jcfranco Sep 18, 2023

ashetland Sep 18, 2023

jcfranco commented Sep 19, 2023

feat(alert): make component responsive #7755

feat(alert): make component responsive #7755

Conversation

jcfranco commented Sep 16, 2023 • edited Loading

Summary

Additional changes

driskull left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcfranco commented Sep 19, 2023

jcfranco commented Sep 16, 2023 •

edited

Loading