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

feat: make ClearButton build again, unblock CCX #1304

Merged
merged 14 commits into from
Nov 5, 2021
Merged

feat: make ClearButton build again, unblock CCX #1304

merged 14 commits into from
Nov 5, 2021

Conversation

lazd
Copy link
Member

@lazd lazd commented Nov 1, 2021

Description

How and where has this been tested?

  • How this was tested:
  • Browser(s) and OS(s) this was tested with:

Screenshots

To-do list

  • Make ClearButton build
  • Make Tag use ClearButton
  • Make Search/SearchWithin use ClearButton
  • Update to [email protected]
  • Fix tag
  • Make an overBackground or staticColor ClearButton for Toast and Tag - used only in Toast
  • Decide whether we manually set the color of the ClearButton icon in Tag, or we tell people to add the overBackground class - Manually setting
  • Merge and release DSS PR https://git.corp.adobe.com/Spectrum/dss/pull/362
  • Validate everything @GarthDB @pfulton

@lazd lazd changed the title feat: implement InfieldButton, Textfield, and some other goodies for Express feat: make ClearButton build again, unblock CCX Nov 2, 2021
@lazd lazd requested a review from GarthDB November 2, 2021 19:29
@lazd
Copy link
Member Author

lazd commented Nov 3, 2021

@GarthDB Did you happen to run the VRT? Let's get this one in ASAP.

@pfulton
Copy link
Collaborator

pfulton commented Nov 3, 2021

I ran VRT locally, and got 25 failures. For several of them, it looks like it failed because the clear button was added in places where it previously didn't exist.

However, the positioning in the Toast component looks off:

Screen Shot 2021-11-03 at 3 14 35 PM

@lazd
Copy link
Member Author

lazd commented Nov 3, 2021

Ok @pfulton we need to merge and release https://git.corp.adobe.com/Spectrum/dss/pull/362, then this PR should be ready.

You will still see VRT issues for Tooltip on this branch at this moment because the DSS fixes for Tooltip label padding are not present yet.

@GarthDB
Copy link
Member

GarthDB commented Nov 4, 2021

Tooltip has changed:
Reference:
image

Test:
image

@lazd
Copy link
Member Author

lazd commented Nov 4, 2021

@pfulton fixed that last issue, once VRT completes here I think we can merge this and be off to the races with CCX!

@lazd
Copy link
Member Author

lazd commented Nov 4, 2021

@GarthDB, yup Tooltip changed. During workshop, @badimon told us that "Tooltip has the same shape as ActionButton and no longer has a custom icon size or padding," so we made it inherit and it changed accordingly. Looks much nicer now!

@GarthDB
Copy link
Member

GarthDB commented Nov 4, 2021

Same with Toast with clear button:
Reference:
image

Test:
image

@lazd
Copy link
Member Author

lazd commented Nov 4, 2021

@GarthDB yup that's actually Toast in the wrapping example page, bb014ca fixes it.

@lazd lazd requested a review from badimon November 4, 2021 23:50
@lazd
Copy link
Member Author

lazd commented Nov 4, 2021

The docs site has been deployed to https://git.corp.adobe.com/pages/lawdavis/spectrum-css-ccx-verify/docs/clearbutton.html for verification. Thanks @badimon!

@lazd lazd merged commit ae9399a into main Nov 5, 2021
@GarthDB GarthDB deleted the express branch November 8, 2021 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants