feat(overlay,commons): deprecate overlay; migrate references to commons #2429
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
castastrophe
force-pushed
the
feat-deprecate-overlay-migrate-commons
branch
from
6879a96
to
76ab549
Compare
castastrophe
added
run_vrt
castastrophe
force-pushed
the
feat-deprecate-overlay-migrate-commons
branch
from
76ab549
to
669b620
Compare
File metricsSummaryTotal size: 3.91 MB*
Detailsaccordion
actionbar
actionbutton
assetlist
badge
breadcrumb
button
calendar
card
checkbox
clearbutton
closebutton
colorarea
colorhandle
combobox
datepicker
dial
dialog
divider
dropindicator
dropzone
fieldlabel
helptext
illustratedmessage
infieldbutton
link
logicbutton
menu
modal
picker
popover
progressbar
radio
rating
search
sidenav
slider
splitview
statuslight
steplist
stepper
swatch
switch
table
tabs
tag
textfield
toast
tooltip
typography
underlay
* Results are not gzipped or minified. * An ASCII character in UTF-8 is 8 bits or 1 byte. |
|
🚀 Deployed on https://pr-2429--spectrum-css.netlify.app |
castastrophe
force-pushed
the
feat-deprecate-overlay-migrate-commons
branch
4 times, most recently
from
1c282af
to
cf5da86
Compare
castastrophe
force-pushed
the
feat-deprecate-overlay-migrate-commons
branch
from
cf5da86
to
eb4dd62
Compare
mdt2
approved these changes
Jan 16, 2024
Yes, good question. My plan was to mark the package deprecated in npm. Since it only contains placeholders and no usable CSS, that should be enough to let anyone who might be using it downstream to either migrate off or switch to @spectrum-css/commons. |
castastrophe
force-pushed
the
feat-deprecate-overlay-migrate-commons
branch
from
eb4dd62
to
60a008a
Compare
|
|
castastrophe
force-pushed
the
feat-deprecate-overlay-migrate-commons
branch
from
60a008a
to
d7e08ec
Compare
castastrophe
commented
Jan 16, 2024
castastrophe
force-pushed
the
feat-deprecate-overlay-migrate-commons
branch
8 times, most recently
from
0c23d88
to
4c6fe0e
Compare
jawinn
approved these changes
Jan 16, 2024
There was a problem hiding this comment.
This is really helpful in cleaning up Overlay and removing the duplicate code! Deprecating the component feels like the right way to go.
I think a step 2 for followup refactor work could be to do one of the following:
a. remove overlay entirely from commons and bring its necessary CSS directly to the components that need it. %spectrum-overlay and %spectrum-overlay--open are not providing a lot.
b. OR, at the least removing %spectrum-overlay--bottom--open, %spectrum-overlay--top--open, %spectrum-overlay--right--open, %spectrum-overlay--left--open, which as you've noted are either not used or only used once.
3 tasks
castastrophe
force-pushed
the
feat-deprecate-overlay-migrate-commons
branch
12 times, most recently
from
859ec89
to
6e0ebdd
Compare
- Move from yarn cache:clean to yarn nx reset for clarity and to prevent unexpected regressions when changes made to package.json - Additional comment blocks in production workflow - Add size change reporting - should add details to the build console if we want to see changes to main - Remove built asset download for publish site as it's duplicative & add the storybook fallback task to after the site build so it isn't overwritten by the site clean
castastrophe
force-pushed
the
feat-deprecate-overlay-migrate-commons
branch
from
6e0ebdd
to
ac87071
Compare
pfulton
approved these changes
Jan 17, 2024
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
@spectrum-css/overlay and @spectrum-css/commons contain the same overlay extends. This PR aims to deduplicate this work and unify extends under the commons package, deprecating the @spectrum-css/overlay package.
This also migrates to
postcss-extendinstead ofpostcss-inherit. During the build rework, we ran into a lot of compatibility issues usingpostcss-inheritwith postcss 8 that were blocking our migration and update.@spectrum-css/commons
commons/overlay-coretokens.cssandcommons/basebutton-coretokens.csstocommons/*.csswithout the-coretokens.csspostfix because the non-coretokens assets are no longer used in the library.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.
Components this impacts:
updates to extend basebutton.css from commons
updates to extend overlay.css from commons
other related changes
Validation steps
Regression testing
Validate:
To-do list