New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(alertbanner): migrate to spectrum 2 #2652
base: spectrum-two
Are you sure you want to change the base?
Conversation
Includes new color values for Spectrum 2
BREAKING CHANGE: migrates `Action Group` to Spectrum 2, Removes some mod properties
BREAKING CHANGE: migrates Button Group to Spectrum 2 Also: * docs(buttongroup): expand chromatic coverage * refactor(buttongroup): remove extra css classes
BREAKING CHANGE: uses Spectrum 2 tokens
- @spectrum-css/preview@7.4.2-next.0 - @spectrum-css/actiongroup@5.0.0-next.0 - @spectrum-css/buttongroup@7.0.0-next.0 - @spectrum-css/tokens@14.0.0-next.0 - @spectrum-css/bundle-builder@6.3.1-next.0 - @spectrum-css/component-builder-simple@5.0.1-next.0 - @spectrum-css/component-builder@6.0.1-next.0
BREAKING CHANGE: migrates Close Button to Spectrum 2 Additionally: * test: increase chromatic coverage * fix(closebutton): pass staticColor as arg for SB display * chore(closebutton): remove themes dir * docs(closebutton): adds s2 migration notes * chore(closebutton): specify s2 tokens release for dependency --------- Co-authored-by: Patrick Fulton <pfulton@adobe.com>
* chore: migrate gray-50 to gray-25 Migrates any instance of `--spectrum-gray-50` to use `--spectrum-gray-25` as per the S2 migration guide * chore: migrate gray-75 to gray-50 Migrates usages of `--spectrum-gray-75` to use `--spectrum-gray-50` as per the s2 migration guide. * chore: migrate gray-100 to gray-75 Migrates usages of `--spectrum-gray-100` to use `--spectrum-gray-75` as per the s2 migration guide * chore: migrate gray-200 to gray-100 Migrates usages of `--spectrum-gray-200` to use `--spectrum-gray-100` as per the s2 migration guide * chore: migrate gray-300 to gray-200 Migrates usages of `--spectrum-gray-300` to use `spectrum-gray-200` as per the s2 migration guide * chore(infieldbutton): gray-300 to gray-200
* fix(commons): remove renamed mods marked for deprecation Remove mods that were renamed and previously marked for deprecation, and regenerate mods lists. This will help in reviewing the accuracy of other components' mods lists as they are being migrated to s2. * docs: migration guide notes for mod property deprecations Add notes to components affected by mod property changes in the commons basebutton. * docs(closebutton): updated docs for migration guide and icon size - Removes the "Icon size" variant options, as noted in the closebutton PR. Per Figma changelog "removed icon size as a property". - Updates migration guide with a more organized history. * fix(commons): remove another mod referencing global token Deprecates an additional mod name that was referencing a global token, and updates migration notes for all affected components.
BREAKING CHANGE: uses Spectrum 2 tokens Also: * feat(fieldlabel): medium as default and 100 sizing tokens - Use the correct "100" sized tokens for medium. - Make medium styles the default, to conform with existing pattern. .spectrum-FieldLabel--sizeM class removed as it is no longer necessary. * feat(fieldlabel): use correct font size tokens The font-size tokens used for large and extra large needed to be shifted up to match the s2 spec. * refactor(fieldlabel): remove deprecated mods Remove renamed mods marked as deprecated for S2 release. * docs(fieldlabel): spectrum 2 noted in migration notes Updates some docs and adds migration notes. Clarifies some docs around "left" and "right", and fixes duplicate ID warnings from examples code. * docs(fieldlabel): form - docs update and regenerate mods Add migration guide section to "Form", that currently lives within the Field label component. Regenerates mods to include the removal of deprecated renamed mods. * fix(fieldlabel): rename mod property referencing global token A mod custom property was referencing a global token instead of the component itself. Renames this and adds a note to the migration guide. * feat(fieldlabel): use correct sizing tokens Use correct tokens for some large and extra large custom properties, to matching with design. * style(fieldlabel): applying linter formatting adjustments Run linter fix on the fieldlabel component, which rearranges a few declarations. * docs(fieldlabel): document asterisk icon size Add documentation about asterisk UI icon sizes specified on the design. These sizes are already handled in the Storybook template, but were not really explained to the user. * feat(fieldlabel): black and white static color variants Add classes for static colors, as defined on the designs. And adds examples of these variants to the documentation. * build(fieldlabel): specify s2 tokens release for dependency Use Spectrum 2 version of tokens package in dependencies list.
Takes in the latest beta release for S2 tokens
- @spectrum-css/preview@8.0.5-next.0 - @spectrum-css/accordion@4.2.7-next.0 - @spectrum-css/actionbar@7.2.5-next.0 - @spectrum-css/actionbutton@5.2.7-next.0 - @spectrum-css/actiongroup@5.0.0-next.1 - @spectrum-css/actionmenu@5.1.4-next.0 - @spectrum-css/alertbanner@1.1.42-next.0 - @spectrum-css/alertdialog@1.2.5-next.0 - @spectrum-css/asset@4.0.1-next.0 - @spectrum-css/assetcard@3.1.5-next.0 - @spectrum-css/assetlist@5.2.5-next.0 - @spectrum-css/avatar@6.1.6-next.0 - @spectrum-css/badge@3.2.6-next.0 - @spectrum-css/breadcrumb@8.2.6-next.0 - @spectrum-css/button@12.0.3-next.0 - @spectrum-css/buttongroup@7.0.0-next.1 - @spectrum-css/calendar@4.2.6-next.0 - @spectrum-css/card@7.0.3-next.0 - @spectrum-css/checkbox@8.1.6-next.0 - @spectrum-css/clearbutton@5.1.5-next.0 - @spectrum-css/closebutton@5.0.0-next.0 - @spectrum-css/coachindicator@1.1.6-next.0 - @spectrum-css/coachmark@6.1.6-next.0 - @spectrum-css/colorarea@4.1.6-next.0 - @spectrum-css/colorhandle@7.1.5-next.0 - @spectrum-css/colorloupe@4.2.5-next.0 - @spectrum-css/colorslider@5.1.6-next.0 - @spectrum-css/colorwheel@3.1.6-next.0 - @spectrum-css/combobox@2.1.7-next.0 - @spectrum-css/commons@9.1.3 - @spectrum-css/contextualhelp@2.1.6-next.0 - @spectrum-css/datepicker@2.1.5-next.0 - @spectrum-css/dial@2.2.5-next.0 - @spectrum-css/dialog@9.2.5-next.0 - @spectrum-css/divider@2.2.6-next.0 - @spectrum-css/dropindicator@4.1.5-next.0 - @spectrum-css/dropzone@5.2.6-next.0 - @spectrum-css/fieldgroup@4.2.5-next.0 - @spectrum-css/fieldlabel@8.0.0-next.0 - @spectrum-css/floatingactionbutton@1.2.6-next.0 - @spectrum-css/helptext@4.1.6-next.0 - @spectrum-css/icon@6.0.6-next.0 - @spectrum-css/illustratedmessage@6.1.6-next.0 - @spectrum-css/infieldbutton@4.2.5-next.0 - @spectrum-css/inlinealert@7.1.7-next.0 - @spectrum-css/link@4.2.6-next.0 - @spectrum-css/logicbutton@3.3.5-next.0 - @spectrum-css/menu@6.1.6-next.0 - @spectrum-css/miller@5.1.5-next.0 - @spectrum-css/modal@4.2.7-next.0 - @spectrum-css/opacitycheckerboard@1.1.6-next.0 - @spectrum-css/page@7.1.5-next.0 - @spectrum-css/pagination@7.1.7-next.0 - @spectrum-css/picker@7.2.8-next.0 - @spectrum-css/pickerbutton@4.1.6-next.0 - @spectrum-css/popover@6.2.6-next.0 - @spectrum-css/progressbar@3.1.6-next.0 - @spectrum-css/progresscircle@2.2.4-next.0 - @spectrum-css/radio@8.1.6-next.0 - @spectrum-css/rating@4.2.5-next.0 - @spectrum-css/search@6.2.5-next.0 - @spectrum-css/sidenav@4.2.5-next.0 - @spectrum-css/site@4.2.5-next.0 - @spectrum-css/slider@4.3.6-next.0 - @spectrum-css/splitview@4.2.5-next.0 - @spectrum-css/statuslight@6.1.7-next.0 - @spectrum-css/steplist@4.1.5-next.0 - @spectrum-css/stepper@5.1.6-next.0 - @spectrum-css/swatch@5.1.6-next.0 - @spectrum-css/swatchgroup@2.1.6-next.0 - @spectrum-css/switch@4.2.6-next.0 - @spectrum-css/table@5.2.6-next.0 - @spectrum-css/tabs@4.1.5-next.0 - @spectrum-css/tag@8.1.6-next.0 - @spectrum-css/taggroup@4.1.6-next.0 - @spectrum-css/textfield@6.1.7-next.0 - @spectrum-css/thumbnail@5.2.5-next.0 - @spectrum-css/toast@9.1.26-next.0 - @spectrum-css/tooltip@5.3.6-next.0 - @spectrum-css/tray@2.2.8-next.0 - @spectrum-css/treeview@9.2.7-next.0 - @spectrum-css/typography@5.1.6-next.0 - @spectrum-css/underlay@3.2.5-next.0 - @spectrum-css/well@4.1.5-next.0 - @spectrum-css/tokens@14.0.0-next.2 - @spectrum-css/bundle-builder@7.0.3-next.0 - @spectrum-css/component-builder-simple@5.0.6-next.0 - @spectrum-css/component-builder@7.0.3-next.0
* feat(downstate): docs + implementation for example components * docs: update mdx * docs: reorg, stories live within foundations * docs: decorator for down state dimension tokens * docs: fix mdx hierarchy console error * fix: small iconOnly button gets min perspective * docs: use markdown, update language * fix: disabled, readonly checkbox doesnt have down state * chore(button,checkbox): update package versions
* feat(tokens): use spectrum tokens beta 21 * chore(tokens): add custom token for corner rounding
- @spectrum-css/preview@8.1.0-next.0 - @spectrum-css/accordion@4.2.7-next.1 - @spectrum-css/actionbar@7.2.5-next.1 - @spectrum-css/actionbutton@5.2.7-next.1 - @spectrum-css/actiongroup@5.0.0-next.2 - @spectrum-css/actionmenu@5.1.4-next.1 - @spectrum-css/alertbanner@1.1.42-next.1 - @spectrum-css/alertdialog@1.2.5-next.1 - @spectrum-css/asset@4.0.1-next.1 - @spectrum-css/assetcard@3.1.5-next.1 - @spectrum-css/assetlist@5.2.5-next.1 - @spectrum-css/avatar@6.1.6-next.1 - @spectrum-css/badge@3.2.6-next.1 - @spectrum-css/breadcrumb@8.2.6-next.1 - @spectrum-css/button@14.0.0-next.3 - @spectrum-css/buttongroup@7.0.0-next.2 - @spectrum-css/calendar@4.2.6-next.1 - @spectrum-css/card@7.0.3-next.1 - @spectrum-css/checkbox@14.0.0-next.3 - @spectrum-css/clearbutton@5.1.5-next.1 - @spectrum-css/closebutton@5.0.0-next.1 - @spectrum-css/coachindicator@1.1.6-next.1 - @spectrum-css/coachmark@6.1.6-next.1 - @spectrum-css/colorarea@4.1.6-next.1 - @spectrum-css/colorhandle@7.1.5-next.1 - @spectrum-css/colorloupe@4.2.5-next.1 - @spectrum-css/colorslider@5.1.6-next.1 - @spectrum-css/colorwheel@3.1.6-next.1 - @spectrum-css/combobox@2.1.7-next.1 - @spectrum-css/commons@9.1.4-next.0 - @spectrum-css/contextualhelp@2.1.6-next.1 - @spectrum-css/datepicker@2.1.5-next.1 - @spectrum-css/dial@2.2.5-next.1 - @spectrum-css/dialog@9.2.5-next.1 - @spectrum-css/divider@2.2.6-next.1 - @spectrum-css/dropindicator@4.1.5-next.1 - @spectrum-css/dropzone@5.2.6-next.1 - @spectrum-css/fieldgroup@4.2.5-next.1 - @spectrum-css/fieldlabel@8.0.0-next.1 - @spectrum-css/floatingactionbutton@1.2.6-next.1 - @spectrum-css/helptext@4.1.6-next.1 - @spectrum-css/icon@6.0.6-next.1 - @spectrum-css/illustratedmessage@6.1.6-next.1 - @spectrum-css/infieldbutton@4.2.5-next.1 - @spectrum-css/inlinealert@7.1.7-next.1 - @spectrum-css/link@4.2.6-next.1 - @spectrum-css/logicbutton@3.3.5-next.1 - @spectrum-css/menu@6.1.6-next.1 - @spectrum-css/miller@5.1.5-next.1 - @spectrum-css/modal@4.2.7-next.1 - @spectrum-css/opacitycheckerboard@1.1.6-next.1 - @spectrum-css/page@7.1.5-next.1 - @spectrum-css/pagination@7.1.7-next.1 - @spectrum-css/picker@7.2.8-next.1 - @spectrum-css/pickerbutton@4.1.6-next.1 - @spectrum-css/popover@6.2.6-next.1 - @spectrum-css/progressbar@3.1.6-next.1 - @spectrum-css/progresscircle@2.2.4-next.1 - @spectrum-css/radio@8.1.6-next.1 - @spectrum-css/rating@4.2.5-next.1 - @spectrum-css/search@6.2.5-next.1 - @spectrum-css/sidenav@4.2.5-next.1 - @spectrum-css/site@4.2.5-next.1 - @spectrum-css/slider@4.3.6-next.1 - @spectrum-css/splitview@4.2.5-next.1 - @spectrum-css/statuslight@6.1.7-next.1 - @spectrum-css/steplist@4.1.5-next.1 - @spectrum-css/stepper@5.1.6-next.1 - @spectrum-css/swatch@5.1.6-next.1 - @spectrum-css/swatchgroup@2.1.6-next.1 - @spectrum-css/switch@4.2.6-next.1 - @spectrum-css/table@5.2.6-next.1 - @spectrum-css/tabs@4.1.5-next.1 - @spectrum-css/tag@8.1.6-next.1 - @spectrum-css/taggroup@4.1.6-next.1 - @spectrum-css/textfield@6.1.7-next.1 - @spectrum-css/thumbnail@5.2.5-next.1 - @spectrum-css/toast@9.1.26-next.1 - @spectrum-css/tooltip@5.3.6-next.1 - @spectrum-css/tray@2.2.8-next.1 - @spectrum-css/treeview@9.2.7-next.1 - @spectrum-css/typography@5.1.6-next.1 - @spectrum-css/underlay@3.2.5-next.1 - @spectrum-css/well@4.1.5-next.1 - @spectrum-css/tokens@14.0.0-next.3 - @spectrum-css/bundle-builder@7.0.3-next.1 - @spectrum-css/component-builder-simple@5.0.6-next.1 - @spectrum-css/component-builder@7.0.3-next.1
…2559) * feat(cornerrounding): documentation, custom tokens - feat(cornerrounding): add custom token - chore(cornerrounding): adds dist - docs(cornerrounding): add file strucutre for foundations - feat(cornerrounding): override token value - Sets custom var to override token spectrum-corner-radius-full - chore(cornerrounding): add override to dist - Adds custom var to override token value to dist - chore(cornerrounding): adjust file structure - Also renames file - feat(cornerrounding): add Foundations story for corner-rounding - feat(cornerrounding): add action buttons and checkboxes - Adds action buttons and checkboxes stories to demonstrate corner rounding - docs(cornerrounding): update corner-rounding docs - better utilize storybook features - clean up checkbox and action button stories - hide stories in corner-rounding directory - improve table styling and css token use - improve corner rounding documentation - add tables for component size tokens - fix(cornerrounding): remove mdx for tables - fix(cornerrounding): use sentence case - feat(cornerrounding): use custom token on close button - docs: add alias note - docs: remove reference to non-alias tokens - feat(actionbutton): partial migration for corner rounding only - feat(checkbox): partial migration for corner rounding only - chore(closebutton): update custom var post-refactor - fix: token rebase issue - chore(actionbutton,checkbox): update package version - chore: use corner-radius alias token - chore: update tokens version - chore: revert unneeded component version change - docs: design requested updates, show token with example - docs: move tables to stories, inline spacing for tables * fix: includes error --------- Co-authored-by: Melissa Thompson <mthompson@heysparkbox.com>
Remove dates from in front of version numbers on migration guides, as we're adjusting to this format going forward. Includes removal of the placeholder "x/x/2024" dates that were added. The button component was excluded from this update as they're already removed in the button s2 migration PR.
* feat(opacitycheckerboard): small t-shirt size squares * docs(opacitycheckerboard): chromatic coverage for t-shirt size
🚀 Deployed on https://pr-2652--spectrum-css.netlify.app |
File metricsSummaryTotal size: 4.55 MB* Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.
Detailsalertbanner
* Results are not gzipped or minified. * An ASCII character in UTF-8 is 8 bits or 1 byte. |
d005657
to
366139c
Compare
inline-size: var(--mod-alert-banner-icon-size, var(--spectrum-alert-banner-icon-size)); | ||
block-size: var(--mod-alert-banner-icon-size, var(--spectrum-alert-banner-icon-size)); | ||
margin-block-start: var(--mod-alert-banner-top-icon, var(--spectrum-alert-banner-top-icon)); | ||
--mod-icon-size: var(--mod-alert-banner-icon-size, var(--spectrum-alert-banner-icon-size)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mod passthrough for icon size is here rather than on .spectrum-AlertBanner
so that it doesn't affect Button in the case it also uses an icon (or if there was an icon used in the content).
}, | ||
control: { type: "text" }, | ||
}, | ||
variant: { | ||
name: "Background color variants", | ||
name: "Semantic variants", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are called this on the Guidelines for Alert banner.
components/alertbanner/index.css
Outdated
/* @todo replace with font-size specific line-height token when one is available. results in a line-height of 18px with font-size-100. */ | ||
--spectrum-alert-banner-line-height: 1.28571428; /* stylelint-disable number-max-precision */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no line-height defined for this component in the specs currently. Not setting one was resulting in a different line-height between Chrome and Firefox because they have different defaults. This is to match the 18px measured on the design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good! I'm excited to see it with the new icons, I think the alignment might look a lot different/better 😁
--spectrum-alert-banner-text-margin-block-start: max(0px, | ||
var(--mod-alert-banner-top-to-text, var(--spectrum-alert-banner-top-to-text)) - | ||
var(--mod-alert-banner-block-edge-to-button, var(--spectrum-alert-banner-block-edge-to-button)) | ||
); | ||
--spectrum-alert-banner-text-margin-block-end: max(0px, | ||
var(--mod-alert-banner-bottom-to-text, var(--spectrum-alert-banner-bottom-to-text)) - | ||
var(--mod-alert-banner-block-edge-to-button, var(--spectrum-alert-banner-block-edge-to-button)) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you talk me through this? I wonder if we need some comments here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some comments above this CSS. Basically, the spacing above and below the button was moved to the AlertBanner-body, similar to how Toast has its Toast-body and Toast-content.
This was to try and solve the issue of too much vertical spacing appearing between the text and the button when it wraps. While using the different vertical spacing to edge tokens defined for the text and the button, as well as the spacing-100 defined between the text and button. On prod there was too much vertical spacing between the two:
isOpen: { | ||
name: "Open", | ||
description: "Determines whether the banner is visible or hidden.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we need this 🤔 Clicking the close button doesn't do anything and toggling this doesn't show an animation or anything so I'm not sure it's useful
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's interesting that the Alert banner is display: none
by default, and only displays if the is-open
class is present. Do you think it would be worth keeping the control help make sure this is clear? How about adding an example to the Chromatic-only template without the is-open
class, to make sure that it remains display:none
in VRTs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added that hidden alert banner to the stories (in the code you'll see "This alert banner should never be displayed in VRTs...").
4d9e185
to
a81c712
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking really nice and I don't have a lot to comment on this one other than echoing @mdt2's comments.
If I had to pinpoint one thing that might be improved, this kitchen sink testing preview does look a bit crowded, mainly between the text and alert banners:
I notice that other components (like Button and Action Button) do have a bit more spacing in their testing previews and bringing that to AlertBanner as well might help things feel a bit more uniform.
Adjust some spacings to match the Spectrum 2 spec spacing. And renames some mods so they are more consistent with current naming conventions, and their purpose is more clear. === Update spacing to match the Spectrum 2 spec === One issue with the S1 version was that there was too much space between the text and the button, when the button wrapped to the next line. And there was a different token defined for the space from the bottom edge to the text, and vertically between the button and the text. This helps resolve this by moving some of the spacing to padding on the AlertBanner-body, and subtracting this value from some of the other custom properties. Along with using both column-gap and row-gap properties.
366139c
to
2e91714
Compare
|
Improves spacing in Storybook template, especially the Chromatic only template. Decorator and wrapper styles are modeled after those used in Action button, so they look more similar. Previously the layout was a little cramped and the "detail" elements were missing their margins because they were display inline.
2e91714
to
ce25a3e
Compare
Agreed! It appears that the Details typography element by default is |
f3dd9ff
to
9931a8e
Compare
6dbc850
to
ae7aedd
Compare
e0e0fd2
to
698e904
Compare
Description
Alert banner now uses Spectrum 2 tokens and specifications. In this new design, the divider has been removed. Some spacing has been updated to match the specs.
TO-DO before final review and merge:
alert-banner-top-to-workflow-icon
value. This will move the icon up slightly to align better with the text.The following mod custom properties have been changed:
--mod-alert-banner-size
was renamed to--mod-alert-banner-inline-size
--mod-alert-banner-neutral-background
was previously misspelled. The mod--mod-alert-banner-background
was also added, which will take precendence over the variant background mods.--mod-alert-banner-top-text
was renamed to--mod-alert-banner-top-to-text
--mod-alert-banner-top-icon
was renamed to--mod-alert-banner-top-to-icon
--mod-alert-banner-bottom-text
was renamed to--mod-alert-banner-bottom-to-text
--mod-alert-banner-start-edge
was renamed to--mod-alert-banner-inline-start-to-content
--mod-alert-banner-edge-to-button
was renamed to--mod-alert-banner-block-edge-to-button
--mod-alert-banner-close-button-to-inline-end
and--mod-alert-banner-close-button-to-content
. The previous--mod-alert-banner-close-button-spacing
has been removed.CSS-704
How and where has this been tested?
Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.
Validation steps
updateArgs
. Refresh the page and it will render properly).Regression testing
Validate:
Screenshots
WIP
To-do list