chore: dialog t-shirt sizing #3322

marissahuysentruyt · 2024-10-24T16:23:57Z

Description

This PR aligns the sizing classes for the dialog component to t-shirt sizing, as most of our other components have done in recent years. The old size modifier classes will be deprecated and removed in S2, so deprecation comments were left in the CSS, along with the addition of the t-shirt classes.

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 to run locally, or use the deploy preview.
Verify that the old classes are being applied to dialogs, as before. This change introduces NO markup/template/class name changes.
Pick a dialog to inspect in the browser. Remove the old class name and add the new class. Ensure that the styles do not change between these classes:

old class name	new class name
spectrum-Dialog--small	spectrum-Dialog--sizeS
spectrum-Dialog--medium	spectrum-Dialog (the default, so no size is necessary)
spectrum-Dialog--large	spectrum-Dialog--sizeL

Regression testing

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

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

changeset-bot · 2024-10-24T16:24:01Z

🦋 Changeset detected

Latest commit: 93f309f

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/dialog	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

github-actions · 2024-10-24T16:27:42Z

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

github-actions · 2024-10-24T16:28:47Z

File metrics

Summary

Total size: 4.30 MB^*
Total change (Δ): 🔴 ⬆ 0.12 KB (0.00%)

Table reports on changes to a package's main file. Other changes can be found in the collapsed Details section below.

Package	Size	Δ
dialog	14.68 KB	🔴 ⬆ 0.04 KB

Details

dialog

Filename	Head	Compared to base
index-base.css	14.68 KB	🔴 ⬆ 0.04 KB (0.27%)
index-vars.css	14.68 KB	🔴 ⬆ 0.04 KB (0.27%)
index.css	14.68 KB	🔴 ⬆ 0.04 KB (0.27%)

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

marissahuysentruyt · 2024-10-24T16:33:08Z

components/dialog/index.css

-	/* Be no bigger than max-width,  but also be 90% if the viewport is smaller than max-width */
-	inline-size: fit-content;
+	/* Be no bigger than max-width, but also be 90% if the viewport is smaller than max-width */
+	inline-size: var(--mod-dialog-confirm-medium-width, var(--spectrum-dialog-confirm-medium-width));


I updated this property to match the .spectrum-Dialog--medium definition. There are no --sizeM specific styles in S2- medium is the default so all of the "medium" styles are just under .spectrum-Dialog.

Does that change make sense to do here?

This is fine since .spectrum-Dialog--medium is getting deprecated anyway, but since .spectrum-Dialog and .spectrum--medium would, in theory, have all the same styles, is there a good reason to keep the .spectrum--medium selector in a section by itself? What are your thoughts on moving it up and combining it with the .spectrum-Dialog selector? So until it's deprecated, the selector would be

.spectrum-Dialog, .spectrum-Dialog--medium {

Yes, I like that much better.

rise-erpelding

This is fine as is! I left my thoughts on .spectrum-Dialog--medium but will leave the final decision to you!

rise-erpelding · 2024-10-28T14:26:54Z

components/dialog/index.css

-	/* Be no bigger than max-width,  but also be 90% if the viewport is smaller than max-width */
-	inline-size: fit-content;
+	/* Be no bigger than max-width, but also be 90% if the viewport is smaller than max-width */
+	inline-size: var(--mod-dialog-confirm-medium-width, var(--spectrum-dialog-confirm-medium-width));


This is fine since .spectrum-Dialog--medium is getting deprecated anyway, but since .spectrum-Dialog and .spectrum--medium would, in theory, have all the same styles, is there a good reason to keep the .spectrum--medium selector in a section by itself? What are your thoughts on moving it up and combining it with the .spectrum-Dialog selector? So until it's deprecated, the selector would be

.spectrum-Dialog, .spectrum-Dialog--medium {

cdransf

Looks good to me!

- adds deprecation notice for --small/--medium/--large class names, in favor of the t-shirt sizing class names. - create changeset - updates metadata.json

marissahuysentruyt added size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. s1 labels Oct 24, 2024

marissahuysentruyt self-assigned this Oct 24, 2024

marissahuysentruyt commented Oct 24, 2024

View reviewed changes

marissahuysentruyt force-pushed the marissahuysentruyt/css-813-dialog-t-shirt-classes branch from fff09f9 to bce15e6 Compare October 24, 2024 16:35

marissahuysentruyt added deprecation Flag PRs or issues with this to indicate a component is being deprecated. and removed s1 deprecation Flag PRs or issues with this to indicate a component is being deprecated. labels Oct 24, 2024

marissahuysentruyt force-pushed the marissahuysentruyt/css-813-dialog-t-shirt-classes branch from bce15e6 to de8de1e Compare October 24, 2024 17:05

marissahuysentruyt marked this pull request as ready for review October 24, 2024 17:45

rise-erpelding self-requested a review October 28, 2024 13:50

rise-erpelding approved these changes Oct 28, 2024

View reviewed changes

marissahuysentruyt force-pushed the marissahuysentruyt/css-813-dialog-t-shirt-classes branch from de8de1e to a549d02 Compare October 29, 2024 15:31

rise-erpelding approved these changes Oct 29, 2024

View reviewed changes

marissahuysentruyt added the ready-for-review label Oct 29, 2024

cdransf approved these changes Oct 31, 2024

View reviewed changes

chore: dialog t-shirt sizing

93f309f

- adds deprecation notice for --small/--medium/--large class names, in favor of the t-shirt sizing class names. - create changeset - updates metadata.json

marissahuysentruyt force-pushed the marissahuysentruyt/css-813-dialog-t-shirt-classes branch from a549d02 to 93f309f Compare October 31, 2024 18:16

marissahuysentruyt merged commit ffc6895 into main Oct 31, 2024
12 checks passed

marissahuysentruyt deleted the marissahuysentruyt/css-813-dialog-t-shirt-classes branch October 31, 2024 18:25

github-actions bot mentioned this pull request Oct 31, 2024

chore: release #3358

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: dialog t-shirt sizing #3322

chore: dialog t-shirt sizing #3322

marissahuysentruyt commented Oct 24, 2024

changeset-bot bot commented Oct 24, 2024 •

edited

Loading

github-actions bot commented Oct 24, 2024 •

edited

Loading

github-actions bot commented Oct 24, 2024 •

edited

Loading

dialog

marissahuysentruyt Oct 24, 2024 •

edited

Loading

rise-erpelding Oct 28, 2024

marissahuysentruyt Oct 29, 2024

rise-erpelding left a comment

rise-erpelding Oct 28, 2024

cdransf left a comment

chore: dialog t-shirt sizing #3322

chore: dialog t-shirt sizing #3322

Conversation

marissahuysentruyt commented Oct 24, 2024

Description

How and where has this been tested?

Validation steps

Regression testing

Screenshots

To-do list

changeset-bot bot commented Oct 24, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Oct 24, 2024 • edited Loading

github-actions bot commented Oct 24, 2024 • edited Loading

File metrics

Summary

dialog

marissahuysentruyt Oct 24, 2024 • edited Loading

Choose a reason for hiding this comment

rise-erpelding Oct 28, 2024

Choose a reason for hiding this comment

marissahuysentruyt Oct 29, 2024

Choose a reason for hiding this comment

rise-erpelding left a comment

Choose a reason for hiding this comment

rise-erpelding Oct 28, 2024

Choose a reason for hiding this comment

cdransf left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Oct 24, 2024 •

edited

Loading

github-actions bot commented Oct 24, 2024 •

edited

Loading

github-actions bot commented Oct 24, 2024 •

edited

Loading

marissahuysentruyt Oct 24, 2024 •

edited

Loading