Spectrum CSS Syncing #191

snowystinger · 2020-02-11T21:10:59Z

Ran diff/merge tool
Textfield and Typography need massive help. I've chosen our implementation over spectrum-css for now

Closes

✅ Pull Request Checklist:

Included link to corresponding Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exist for this component).
Looked at the Accessibility Standard and/or talked to @mijordan about Accessibility for this feature.

📝 Test Instructions:

🧢 Your Team:

Textfield and Typography need massive help. I've chosen our implementation over spectrum-css for now

snowystinger · 2020-02-11T21:17:21Z

packages/@adobe/spectrum-css-temp/components/buttongroup/index.css

@@ -22,6 +22,11 @@ governing permissions and limitations under the License.
    flex-shrink: 0;
  }

+  > .spectrum-Rule--vertical {


I don't know why this is in here... but it's right here https://github.com/adobe/spectrum-css/blob/master/components/buttongroup/index.css#L26 and it totally breaks modules

It's due to this: adobe/spectrum-css@ea7b85f

We would otherwise have to apply inline styles to position the rules correctly.

I'm thinking maybe this doesn't belong in spectrum-css then? and we should handle it in react-spectrum?

Mmmm, I think we would have to make the call -- do we ever try to modify components that appear inside of other components? If we do, we break modules, but things Just Work™️ when placed in different contexts. If we don't some things are a little more manual.

I would be fine with backing out this change, nobody asked for it, I just added it during my work on a related issue.

with the breaking changes, lets apply align-self: stretch to all dividers #198 (comment)
how does that sound? @lazd

snowystinger · 2020-02-11T21:18:24Z

packages/@adobe/spectrum-css-temp/components/dialog/index.css

@@ -115,6 +115,9 @@ governing permissions and limitations under the License.
  .spectrum-Dialog-divider {
    display: none;
  }
+  margin-left: var(--spectrum-dialog-icon-margin-left);
+  flex-shrink: 0;
+  align-self: flex-start;


not sure we need these...

Those bits were added by @jianliao and I to fix the positioning of the icon, but they have somehow appeared in the wrong place?

They are supposed to be part of .spectrum-Dialog-typeIcon. See adobe/spectrum-css@1d56fd9

thanks for the catch, doing these merges causes my eyes to bleed and sometimes i miss things
I'll fix it tomorrow

snowystinger · 2020-02-11T21:18:56Z

packages/@adobe/spectrum-css-temp/components/menu/index.css

@@ -122,13 +122,27 @@ governing permissions and limitations under the License.
  .spectrum-Icon + .spectrum-Menu-itemLabel,
  .spectrum-Menu-itemIcon  + .spectrum-Menu-itemLabel {
    margin-inline-start: var(--spectrum-selectlist-thumbnail-image-padding-x);
+
+    inline-size: calc(


do we need this?

Hmm this looks like it was from spectrum-css adding overflow-wrap for MenuItems (german connected nouns and stuff). I think we'll need this when we get variable sized v3 MenuItem support, though we'll have to confirm what the design should look like since the XD file I have doesn't actually show multiline labels or descriptions

Should it be included here right now in this update?

I think we can discard it for now since we don't have support for variable sized menu items

This sizes the labels with icons so they wrap correctly. If it's not hurting anything, I say leave it.

snowystinger · 2020-02-11T21:19:27Z

packages/@adobe/spectrum-css-temp/components/popover/index.css

+.spectrum-Popover {
+  &.spectrum-Popover--dialog {
+    .spectrum-Dialog-typeIcon {
+      margin-left: var(--spectrum-popover-dialog-icon-margin-left);


unsure about this, also it's breaking modules

@jianliao it looks like this shouldn't be here at all; it's already defined in Dialog.

packages/@adobe/spectrum-css-temp/components/statuslight/index.css

snowystinger · 2020-02-11T21:20:42Z

packages/@adobe/spectrum-css-temp/components/treeview/index.css

@@ -87,6 +87,7 @@ governing permissions and limitations under the License.

 .spectrum-TreeView-indicator {
  display: block;
+  box-sizing: content-box;


@dannify I think this is fixing that issue someone had in the channel?

Yup, it was adobe/spectrum-css#467

snowystinger · 2020-02-11T21:21:16Z

packages/@adobe/spectrum-css-temp/components/typography/skin.css

+
+/* t-shirt size-based typography */
+
+@mixin typographyTShirtColor .spectrum-Heading--XXXL {}


@devongovett know anything about these typography changes?

We just dropped a new Typography API: adobe/spectrum-css#210

The old API is still present, so it's a backwards compatible change (and means we have exactly 3 ways of doing Typography, and 2 of them are deprecated. yay.)

More details on migration and whatnot live here: http://spectrum-css-archives.s3-website.us-east-2.amazonaws.com/2.17.0/docs/typography.html#migratingfromdeprecatedtypography

adobe-bot · 2020-02-11T21:54:35Z

Build successful! View the storybook

lazd · 2020-02-12T00:20:54Z

I will manually correct Textfield as part of #162. The breaking change from adobe/spectrum-css#142 is also in my sprint, so I'll be working on that.

lazd · 2020-02-12T00:21:40Z

@berhard-adobe added the new Typography API with t-shirt sizing, I suggest V3 moves to that instead of implementing any part of the old API it's deprecated. See http://spectrum-css-archives.s3-website.us-east-2.amazonaws.com/2.17.0/docs/typography.html

adobe-bot · 2020-02-19T18:20:48Z

Build successful! 🎉

adobe-bot · 2020-02-19T18:41:39Z

Build successful! 🎉

snowystinger · 2020-02-19T19:05:49Z

Ok, I think I've adjusted in response to most of the comments

We'll need to introduce typography, but it wasn't immediately clear how to move over to that, just using the classnames didn't work, they weren't picked up on the css module object. Good news, nothing is implemented with it yet except documentation site.

Dialog Icon may need some placement help, but it's pretty good now

Divider needs some resolution with regards to putting align-self: stretch on all of them

@dannify

adobe-bot · 2020-02-19T19:09:34Z

Build successful! 🎉

adobe-bot · 2020-02-20T23:47:14Z

Build successful! 🎉

snowystinger · 2020-05-22T19:05:43Z

reopen if needed later

Ran diff/merge tool

02399ab

Textfield and Typography need massive help. I've chosen our implementation over spectrum-css for now

snowystinger requested review from dannify and devongovett as code owners February 11, 2020 21:10

snowystinger commented Feb 11, 2020

View reviewed changes

packages/@adobe/spectrum-css-temp/components/statuslight/index.css Show resolved Hide resolved

snowystinger commented Feb 11, 2020

View reviewed changes

fix merge conflict

d2d1196

Merge branch 'master' into spectrum-css-upgrade

700ab67

Fix merge conflict issues

9e88e05

fix dialog icon placement

1526ade

Merge branch 'master' into spectrum-css-upgrade

f3024b1

snowystinger closed this May 22, 2020

snowystinger deleted the spectrum-css-upgrade branch May 22, 2020 19:05

devongovett pushed a commit that referenced this pull request Jul 25, 2024

Adding contextual help stories to docs build (#191)

7b0e2f8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spectrum CSS Syncing #191

Spectrum CSS Syncing #191

snowystinger commented Feb 11, 2020

snowystinger Feb 11, 2020

lazd Feb 12, 2020

snowystinger Feb 12, 2020

lazd Feb 12, 2020

snowystinger Feb 19, 2020

snowystinger Feb 11, 2020

lazd Feb 12, 2020

snowystinger Feb 12, 2020

snowystinger Feb 11, 2020

snowystinger Feb 11, 2020

LFDanLu Feb 11, 2020

snowystinger Feb 11, 2020

LFDanLu Feb 11, 2020

lazd Feb 12, 2020

snowystinger Feb 11, 2020

lazd Feb 12, 2020

snowystinger Feb 11, 2020

lazd Feb 12, 2020

snowystinger Feb 11, 2020

devongovett Feb 11, 2020

lazd Feb 12, 2020

lazd Feb 12, 2020

adobe-bot commented Feb 11, 2020

lazd commented Feb 12, 2020

lazd commented Feb 12, 2020

adobe-bot commented Feb 19, 2020

adobe-bot commented Feb 19, 2020

snowystinger commented Feb 19, 2020 •

edited

Loading

adobe-bot commented Feb 19, 2020

adobe-bot commented Feb 20, 2020

snowystinger commented May 22, 2020


		/* t-shirt size-based typography */

		@mixin typographyTShirtColor .spectrum-Heading--XXXL {}

Spectrum CSS Syncing #191

Spectrum CSS Syncing #191

Conversation

snowystinger commented Feb 11, 2020

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Team:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adobe-bot commented Feb 11, 2020

lazd commented Feb 12, 2020

lazd commented Feb 12, 2020

adobe-bot commented Feb 19, 2020

adobe-bot commented Feb 19, 2020

snowystinger commented Feb 19, 2020 • edited Loading

adobe-bot commented Feb 19, 2020

adobe-bot commented Feb 20, 2020

snowystinger commented May 22, 2020

snowystinger commented Feb 19, 2020 •

edited

Loading