Skip to content

fix(tooltip): restore negative and informative semantic variants #3641

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 3 commits into from
Apr 2, 2025

Conversation

cdransf
Copy link
Member

@cdransf cdransf commented Mar 25, 2025

Description

This restores the negative and informative tooltip semantic variant styles, story controls and test cases. Icons have been removed for all variants as they are not present in the specifications provided by design.

Restored mods

--mod-tooltip-background-color-informative
--mod-tooltip-background-color-negative"

Validation steps

  1. Open the Storybook link for this PR.
  2. Navigate to the Default story for the tooltip component.
  3. Select the informative and negative tooltip styles in controls and verify that they match the Figma design(s).

Screenshots

Screenshot 2025-03-25 at 10 39 32 AM

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. ✨

Sorry, something went wrong.

Copy link

changeset-bot bot commented Mar 25, 2025

🦋 Changeset detected

Latest commit: dfcbd61

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

This PR includes changesets to release 1 package
Name Type
@spectrum-css/tooltip Minor

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

@cdransf cdransf force-pushed the cdransf/s2-tooltip-semantic-variants branch from f6a7e85 to c215a47 Compare March 25, 2025 17:42
@cdransf cdransf marked this pull request as ready for review March 25, 2025 17:43
@cdransf cdransf self-assigned this Mar 25, 2025
@cdransf cdransf added size-3 M ~18-30hrs; moderate effort or complexity, several work days needed. skip_vrt Add to a PR to skip running VRT (but still pass the action) ready-for-review labels Mar 25, 2025
Copy link
Contributor

github-actions bot commented Mar 25, 2025

File metrics

Summary

Total size: 1.37 MB*

Package Size Minified Gzipped
tooltip 31.95 KB 30.70 KB 3.11 KB

tooltip

Filename Head Minified Gzipped Compared to base
index.css 31.95 KB 30.70 KB 3.11 KB 🔴 ⬆ 1.17 KB
metadata.json 20.57 KB - - 🔴 ⬆ 0.37 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.

Copy link
Contributor

github-actions bot commented Mar 25, 2025

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

@cdransf cdransf closed this Mar 25, 2025
@cdransf cdransf reopened this Mar 25, 2025
@cdransf cdransf force-pushed the cdransf/s2-tooltip-semantic-variants branch from c215a47 to d961f08 Compare March 27, 2025 17:49
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.

Yay, thanks for getting these variants back in!

Could you update the test file please? Looks like we still have the positive variant captured, but we can delete it!

Screenshot 2025-04-01 at 2 40 31 PM

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.

I think this looks great!

Just a note that the commit when we squash this down, should probably be a fix, since we're changing CSS, but not introducing a breaking change. Cassondra mentioned once that if there's CSS changes, we should label our commits as fix or feat.

@@ -14,8 +14,6 @@
@import "@spectrum-css/commons/overlay.css";

.spectrum-Tooltip {
--spectrum-tooltip-backgound-color-default-neutral: var(--spectrum-neutral-subdued-background-color-default);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@cdransf cdransf force-pushed the cdransf/s2-tooltip-semantic-variants branch 2 times, most recently from 616a05a to cfa34ed Compare April 1, 2025 22:34
@cdransf cdransf changed the title chore(tooltip): restore negative and informative semantic variants fix(tooltip): restore negative and informative semantic variants Apr 2, 2025
cdransf added 3 commits April 2, 2025 08:18

Verified

This commit was signed with the committer’s verified signature.
cdransf Cory Dransfeldt

Verified

This commit was signed with the committer’s verified signature.
cdransf Cory Dransfeldt

Verified

This commit was signed with the committer’s verified signature.
cdransf Cory Dransfeldt
@cdransf cdransf force-pushed the cdransf/s2-tooltip-semantic-variants branch from cfa34ed to dfcbd61 Compare April 2, 2025 15:19
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 looks great! Thanks for this update and documenting it so well!

"@spectrum-css/tooltip": minor
---

#### S2: restore negative and informative semantic variants
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice use of the heading levels 🥳

@cdransf cdransf merged commit 0b730ea into spectrum-two Apr 2, 2025
12 checks passed
@cdransf cdransf deleted the cdransf/s2-tooltip-semantic-variants branch April 2, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review size-3 M ~18-30hrs; moderate effort or complexity, several work days needed. 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.

None yet

3 participants