Skip to content

Conversation

@jawinn
Copy link
Collaborator

@jawinn jawinn commented Aug 6, 2024

Description

It was reported that this normalize CSS for IE 9/10/11, which Spectrum Web Components converts to the selector :host(:not(:root)), was leading to "clipping on iOS mobile (for Express) as well as on Safari on MacOS". An Express-specific hotfix was already put in to override this overflow declaration.

This is safe to remove as the project does not support IE 11.

Reference:
The original necolas/normalize.css has this normalization labeled as "Correct overflow not hidden in IE 9/10/11."

CSS-700

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

@marissahuysentruyt

  • Icon stories render correctly (recommendation: also check in Safari and Edge)
  • There are no VRT changes

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.

To-do list

@changeset-bot
Copy link

changeset-bot bot commented Aug 6, 2024

🦋 Changeset detected

Latest commit: 879f318

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/icon 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
Copy link
Contributor

github-actions bot commented Aug 6, 2024

File metrics

Summary

Total size: 4.63 MB*
Total change (Δ): ⬇ 0.30 KB (-0.01%)

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

Package Size Δ
icon 11.33 KB ⬇ 0.08 KB

Details

icon

File Head Base Δ
index-base.css 11.33 KB 11.41 KB ⬇ 0.08 KB (-0.67%)
index-vars.css 11.33 KB 11.41 KB ⬇ 0.08 KB (-0.67%)
index.css 11.33 KB 11.41 KB ⬇ 0.08 KB (-0.67%)
metadata.json 5.96 KB 6.02 KB ⬇ 0.06 KB (-1.02%)
* 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.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2024

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

@jawinn jawinn added size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. run_vrt For use on PRs looking to kick off VRT ready-for-review labels Aug 6, 2024
@jawinn jawinn force-pushed the jawinn/css-700-icon-remove-legacy-ie-css branch from b0f116c to f222edb Compare August 6, 2024 15:04
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt left a comment

Choose a reason for hiding this comment

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

I checked both our prod open source icon storybook and the preview url in Chrome, Firefox, Safari and Edge on Mac, then Edge & Firefox on Windows via assistiv labs. I toggled the Express context and Large scale and didn't see any clipping on our end 👍

If I get all the way into the SWC storybook, I found examples of the Express icon clipping and saw the overflow: hidden code. I don't think this fix would be immediately visible via the preview link though (since it wouldn't be released yet for SWC to take in, right?), so I don't think I should be surprised that I still see the clipping. Hopefully this will improve it for them, and in turn Express!

It was reported that this normalize CSS for IE 9/10/11, in SWC which
converts to the selector :host(:not(:root)), was causing problems with
clipping in iOS mobile for Express as well as on Safari on MacOS. This
is safe to remove as the project does not support IE 11.
@castastrophe castastrophe force-pushed the jawinn/css-700-icon-remove-legacy-ie-css branch from f222edb to 879f318 Compare August 12, 2024 20:02
@castastrophe castastrophe enabled auto-merge (squash) August 12, 2024 20:02
@castastrophe castastrophe merged commit fd00178 into main Aug 12, 2024
@castastrophe castastrophe deleted the jawinn/css-700-icon-remove-legacy-ie-css branch August 12, 2024 20:12
@github-actions github-actions bot mentioned this pull request Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review run_vrt For use on PRs looking to kick off VRT size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants