Skip to content

feat(contextual-help): S2 migration #3909

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

Merged
merged 10 commits into from
Jun 20, 2025

Conversation

cdransf
Copy link
Member

@cdransf cdransf commented Jun 6, 2025

Description

CSS-819

S2 migration for contextual help

This migrates the contextual help component to the latest Spectrum 2 designs. Custom properties have been remapped and added per the design specification.

Typographic and color tokens have been updated per design specifications.

All existing popover positioning varations are supported.

New custom properties

--spectrum-contextual-help-body-color
--spectrum-contextual-help-body-line-height
--spectrum-contextual-help-body-sans-serif-font-family
--spectrum-contextual-help-body-sans-serif-font-style
--spectrum-contextual-help-body-sans-serif-font-weight
--spectrum-contextual-help-title-color
--spectrum-contextual-help-title-font-style
--spectrum-contextual-help-title-font-weight
--spectrum-contextual-help-title-line-height
--spectrum-contextual-help-title-sans-serif-font-family

New mods

--mod-contextual-help-body-line-height
--mod-contextual-help-body-sans-serif-font-family
--mod-contextual-help-body-sans-serif-font-style
--mod-contextual-help-body-sans-serif-font-weight
--mod-contextual-help-body-size
--mod-contextual-help-title-color
--mod-contextual-help-title-font-style
--mod-contextual-help-title-font-weight
--mod-contextual-help-title-line-height
--mod-contextual-help-title-sans-serif-font-family
--highcontrast-contextual-help-heading-color
--highcontrast-contextual-help-title-color

Removed mods

--highcontrast-contextual-help-heading-color

Screenshots

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • I have tested these changes in Windows High Contrast mode.
  • If my change impacts other components, I have tested to make sure they don't break.
  • If my change impacts documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

@cdransf cdransf self-assigned this Jun 6, 2025
@cdransf cdransf added size-5 L ~30-42hrs; lots of effort or complexity, most of a sprint needed to complete. wip This is a work in progress, don't judge. skip_vrt Add to a PR to skip running VRT (but still pass the action) labels Jun 6, 2025
Copy link

changeset-bot bot commented Jun 6, 2025

🦋 Changeset detected

Latest commit: 52e6d7a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@spectrum-css/contextualhelp Major
@spectrum-css/bundle Patch
@spectrum-css/preview Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jun 6, 2025

🚀 Deployed on https://pr-3909--spectrum-css.netlify.app

Copy link
Contributor

github-actions bot commented Jun 6, 2025

File metrics

Summary

Total size: 1.42 MB*

Package Size Minified Gzipped
contextualhelp 4.49 KB 4.32 KB 1.00 KB

contextualhelp

Filename Head Minified Gzipped Compared to base
index.css 4.49 KB 4.32 KB 1.00 KB 🔴 ⬆ 2.29 KB
metadata.json 2.72 KB - - 🔴 ⬆ 1.44 KB
* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch from 3bdff09 to 1850227 Compare June 9, 2025 20:11
@cdransf cdransf changed the title WIP(contextual-help): S2 migration feat(contextual-help): S2 migration Jun 9, 2025
@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch from de08037 to 57323f8 Compare June 9, 2025 23:00
@cdransf cdransf marked this pull request as ready for review June 10, 2025 19:38
@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch from c56c432 to f237622 Compare June 10, 2025 19:39
@cdransf cdransf added ready-for-review and removed wip This is a work in progress, don't judge. labels Jun 10, 2025
@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch from f237622 to 40e2b6b Compare June 10, 2025 21:40
Copy link
Collaborator

@castastrophe castastrophe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looking great. Well documented and thanks for the storybook clean-up. I left one small suggestion about how we document high contrast hooks.

@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch 2 times, most recently from 1bceda1 to 75a08f1 Compare June 12, 2025 17:33
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some questions for you!

  1. Is there a reason we have customStyles on every story? Can we get rid of any and use our CSS instead? I haven't actually tried removing them, so we might, I'm not sure.

  2. I couldn't comment on the action button code, but I think we need to gather the globals data so that we can appropriately render both the small action button for large/mobile, and the XS action button for medium/desktop

// before the return statement
const { globals = {} } = context;
const scale = globals.scale ?? "medium"; // this const may not be necessary, so you could refactor this out if you wanted

// then in the ActionButton call, change the size declaration: 
size: scale === "medium" ? "xs" : "s",
  1. Could you double check all of the mods in the CSS? There were quite a few places where I was seeing --mod-spectrum-contextual-help* instead of just --mod-contextual-help*. (places where it was already like that, I mean)
  2. How does contextual help get triggered? Would we want to add isSelected: false to the action button at all, just to match the design, or is this component triggered because an action button was selected?

@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch 6 times, most recently from 6bff760 to 662f926 Compare June 18, 2025 00:00
@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch 2 times, most recently from f0df0a6 to be5d959 Compare June 19, 2025 21:47
@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch 3 times, most recently from 277e230 to a5fd388 Compare June 20, 2025 16:12
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, this is looking really good! I'm going to approve, but I would suggest the custom prop removals Aziz mentioned. I didn't want to request changes again since he already caught them! WHCM looks good too- great work!

@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch from a5fd388 to f73ec5c Compare June 20, 2025 18:07
Copy link
Contributor

github-actions bot commented Jun 20, 2025

📚 Branch preview

PR #3909 has been deployed to Azure Blob Storage: https://spectrumcss.z13.web.core.windows.net/pr-3909/index.html.

Copy link
Contributor

@aramos-adobe aramos-adobe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one more token removal!

--spectrum-contextual-help-border-radius: var(--mod-popover-corner-radius);

I don't think we need this either since you're using --mod-popover-corner-radius then you can remove the border-radius in the spectrum-ContextualHelp-popover selector

@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch from f73ec5c to f059497 Compare June 20, 2025 18:31
@cdransf cdransf requested a review from aramos-adobe June 20, 2025 18:32
@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch from f059497 to 52e6d7a Compare June 20, 2025 21:06
@cdransf cdransf force-pushed the cdransf/s2-migration-contextual-help branch from 52e6d7a to ef68364 Compare June 20, 2025 21:19
@cdransf cdransf enabled auto-merge (squash) June 20, 2025 21:21
@cdransf cdransf merged commit 08b3345 into spectrum-two Jun 20, 2025
19 of 26 checks passed
@cdransf cdransf deleted the cdransf/s2-migration-contextual-help branch June 20, 2025 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review size-5 L ~30-42hrs; lots of effort or complexity, most of a sprint needed to complete. skip_vrt Add to a PR to skip running VRT (but still pass the action)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants