Skip to content
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

docs(progressbar, tooltip): reimplement default args in template #3485

Open
wants to merge 93 commits into
base: spectrum-two
Choose a base branch
from

Conversation

marissahuysentruyt
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt commented Jan 8, 2025

Description

During a different PR review, I noticed that the docs pages for tooltip and progress bar weren't rendering, and had error messages for undefined args. With a little digging, I saw there were missing args in the templates for those 2 components. Adding those back in render the components once again.

Progress bar was just incorrectly named isStaticWhite when the arg was actually staticColor.
Tooltip was missing the variant = "neutral".

Both of the changes were pulled from s2-foundations-redux. No changeset is needed.

Before 🚫

Screenshot 2025-01-08 at 4 52 10 PM Screenshot 2025-01-08 at 4 51 59 PM

After ✅

Screenshot 2025-01-08 at 4 51 21 PM Screenshot 2025-01-08 at 4 51 36 PM

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

  • Pull down the branch or visit the deploy preview
  • Verify the docs pages for progress bar and tooltip render the stories as expected, and no errors occur.
  • Verify the stories pages for progress bar and tooltip render as expected as well, with no errors.
  • Feel free to check the Chromatic tests for both components to make sure everything is hunky-dory there too.

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. If components have been modified, VRTs have been run on this branch:
  • VRTs have been run and looked at.
  • Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

To-do list

  • I have read the contribution guidelines.
  • I have updated relevant storybook stories and templates.
  • ✨ This pull request is ready to merge. ✨

castastrophe and others added 30 commits December 29, 2024 13:29
Expanding the existing themes system to support the new S2 mappings.

---
Co-authored-by: castastrophe <[email protected]>
Co-authored-by: Patrick Fulton <[email protected]>
Co-authored-by: Cory Dransfeldt <[email protected]>
Co-authored-by: Aziz Ramos <[email protected]>
Co-authored-by: Josh Winn <[email protected]>
Co-authored-by: Rise Erpelding <[email protected]>
Co-authored-by: Marissa Huysentruyt <[email protected]>
Co-authored-by: Rajdeep Chandra <[email protected]>
Co-authored-by: TarunAdobe <[email protected]>
Co-authored-by: Dragan Eror<[email protected]>
* feat: s2 foundations non-gray-800 colors update

* add changeset
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* docs(contextualhelp): remove MDX file

- adds some additional documentation regarding stories/variants
- migrates documentation in MDX to stories.js instead

* docs(fieldgroup): remove MDX file

- creates additional templates to showcase variants in a single story on
the docs page
- additional documentation to give context to stories
- corrects sentence-casing in story names
- removes duplicate control for label text
- migrates documentation in MDX to stories.js instead

* chore(fieldgroup,contextualhelp): cleans up test files

- fieldgroup: we don't really need to change the label text or help text
between test cases, so some of the test-specific changes were removed
- contextualhelp: add help icon test case

* docs(datepicker): remove MDX file

- adds missing date picker docs
- migrates documentation in MDX to stories.js instead
- creates OpenClosedTemplate to display both states in a single story on
the docs page
- adds Range variant to the sidebar since that control isn't in the table
* docs(accordion): remove MDX file

- adds accordion sizing story
- migrates documentation in MDX to stories.js instead

* docs(actionbar): remove MDX file

- adds actionbar flexible and sticky variants to default/emphasized
stories
- migrates documentation in MDX to stories.js instead
- updates references to mods tables

* docs(asset): remove MDX file
…e mdx files (#3409)

* docs(colorarea): remove MDX file

- adds links to other color components
- migrates documentation from MDX to stories file

* docs(colorhandle): remove MDX file

- adds links to other color components
- migrates documentation from MDX to stories file

* docs(colorloupe): remove MDX file

- migrates documentation from MDX to stories file

* docs(colorslider): remove MDX file

 - migrates documentation from MDX to stories file

* docs(colorwheel): remove MDX file

- adds missing documentation particularly around the WithColorArea story
- migrates documentation from MDX to stories file

* docs(colorarea): clarify docs on color area handle

* docs(colorslider): add punctuation and reorganize docs
* docs(assetcard): remove MDX file

- adds some missing documentation regarding stories, classes and custom
properties
- corrects sentence-casing of story names

* docs(avatar): remove MDX file

- adds some missing documentation regarding stories
- reorganizes some information to sit with appropriate story/variant
- migrates documentation from MDX file to the stories file instead

* chore(avatar): fix disabled test arguments

* docs(badge): remove MDX file

- adds notice badge variants to semantic story
- adds sizing story to docs page
- migrates documentation in MDX to stories.js instead

* docs(badge): pr fixes

- remove empty doc block line
- remove html wrapper in favor of content array
- add notice badge to test coverage
* chore(card): remove default args from test cases

* docs(card): remove MDX file

- adds some missing documentation regarding stories
- reorganizes some information to sit with appropriate story/variant
- migrates documentation from MDX file to the stories file instead

* docs(coachmark): remove MDX file

- adds some missing documentation regarding stories
- reorganizes some information to sit with appropriate story/variant
- migrates documentation from MDX file to the stories file instead

* chore(coachmark): adds extra chromatic coverage

* docs(calendar): remove MDX file

- migrates documentation from MDX file to the stories file instead

* docs(calendar): clarifies days of the week control name

* docs(card): clarify quiet control requirements

* chore(coachmark): remove html wrapper in favor of content array

* chore(card): fix isQuiet controls and usage in template

* docs(asset,card): isCardAssetOverride renamed to isImageFill
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/[email protected]
 - @spectrum-css/[email protected]
 - @spectrum-css/[email protected]
 - @spectrum-css/[email protected]
 - @spectrum-css/[email protected]
 - @spectrum-css/[email protected]
 - @spectrum-css/[email protected]
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 <[email protected]>
* 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
jawinn and others added 13 commits December 29, 2024 14:10
Icons were not loading on the spectrum-two branch. The fetchIconSVG
function is not working (this function no longer exists in main). This
is a light touch update so icons appear, until spectrum-two is back up
to date with main.
* feat(alertbanner): start migration to spectrum two

Migrates the component to spectrum two.
- Theme files are removed, and their custom properties integrated into
  the main index.css.
- Renames misspelled mod custom property (noted in docs)
- Simplifies background color styles by changing a single custom
  property per variant, as done on other components.

* feat(alertbanner): spectrum 2 spacing and mod name cleanup

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.

=== adjust storybook template spacing and decorator ===

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.

* docs(alertbanner): custom storybook docs page and template updates

Creates a custom MDX docs page in Storybook for the Alert banner
component. And reworks the stories and chromatic-only templates a little
to facilitate and increase coverage.

Includes Storybook VRT coverage for the max width changes in PR #2762 .
Tests the display of the alert banner when in a container larger than
and smaller than its max-inline-size.

* chore(alertbanner): linter updates and update mods

- Add stylelint disable line comments for some false positives caught by
  the linter.
- Re-generate mods file

* docs(alertbanner): use stories and template from main

Use the stories and updated template from main, with a few things left
out that are not yet available in the spectrum-two branch.

* feat(alertbanner): support no close button or no action button

Adds spacing to the end of the alert banner when the close button is
not displayed, or both the close button and action button are not
displayed. These are allowed combinations of options.

* feat(alertbanner): use spec defined line-height token

* feat(alertbanner): add font-family token and mod

* feat(alertbanner): add cjk line-height

* feat(alertbanner): remove unnecessary styles for end element

Remove styles for .spectrum-AlertBanner-end, which were no longer doing
anything after the removal of the divider.
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Storybook foundations pages now display as a single page instead of in
folders with their example stories appearing.
These now use the newer !dev tag to hide them. The old custom
"foundation" tag was previously used to hide them, but this tag is no
longer used.

Also fixes some linter errors encountered during commit; a couple
unused variables and incorrect typeof comparisons.
* fix(search): restore missing custom properties
* fix(colorwheel): restore missing custom properties
* fix(assetcard): restore missing custom properties
* fix(treeview): restore missing custom properties

Restore missing properties from foundations that were flagged
by the linter.

* fix(combobox): restore missing custom properties

Restore missing properties from foundations that were flagged
by the linter. Also removes a group of duplicate custom property
definitions that were flagged.
…3471)

* fix(popover): define --spectrum-popover-border-width in index.css
* fix(well): define --spectrum-well-border-color in index.css
* fix(tabs): define --spectrum-tabs-font-weight
* fix(toast): define default background color and divider color
* fix(tag): define undefined custom properties
* fix(menu): define menu item background colors
* fix(textfield): define undefined custom properties
@marissahuysentruyt marissahuysentruyt added documentation Because documentation is important and shouldn't be broken size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. storybook labels Jan 8, 2025
@marissahuysentruyt marissahuysentruyt self-assigned this Jan 8, 2025
Copy link

changeset-bot bot commented Jan 8, 2025

⚠️ No Changeset found

Latest commit: e3f8cd3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Copy link
Contributor

github-actions bot commented Jan 8, 2025

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

Copy link
Contributor

github-actions bot commented Jan 8, 2025

File metrics

Summary

Total size: 1.73 MB*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@@ -12,6 +12,7 @@ export const Template = ({
rootClass = "spectrum-Tooltip",
label,
placement,
variant = "neutral",
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Jan 8, 2025

Choose a reason for hiding this comment

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

Looks like the informative & negative tooltips got added back to Figma 11/18/24 (approximately). We'll have to create a card to implement the CSS styles and stories (for the docs page & controls)

variant: {
			name: "Variant",
			type: { name: "string" },
			table: {
				type: { summary: "string" },
				category: "Component",
			},
			options: ["neutral", "info", "negative"],
			control: "inline-radio",

tooltip migration PR: https://github.com/adobe/spectrum-css/pull/2743/files#diff-14d4b487bc2de8da9dccaef67bce6cccc92cbce28e2ac16912884edbce436cbc

…tom properties (#3487)

* fix(pickerbutton): add missing custom properties
* fix(radio): add missing custom props
* fix(calendar): add missing custom properties
* fix(stepper): define unused custom props
* fix(progressbar): remove unused custom properties; change background-color to background see #2929; fix static color
* fix(dial): remove unused custom properties; add undefined custom properties
@castastrophe castastrophe force-pushed the marissahuysentruyt/tooltip-progressbar-fix-missing-args branch from 0eca72c to 7823998 Compare January 13, 2025 16:45
castastrophe and others added 2 commits January 14, 2025 15:57
There were missing args in the templates for progress bar and tooltip.
Adding those back in renders the components once again.

Progress bar was just incorrectly named `isStaticWhite` when the arg was
actually `staticColor`. Tooltip was missing the `variant = "neutral"`.
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/tooltip-progressbar-fix-missing-args branch from 7823998 to e3f8cd3 Compare January 16, 2025 17:31
@marissahuysentruyt marissahuysentruyt marked this pull request as ready for review January 16, 2025 17:40
@castastrophe castastrophe force-pushed the spectrum-two branch 2 times, most recently from b438a55 to 46ae687 Compare January 17, 2025 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Because documentation is important and shouldn't be broken size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. storybook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants