Skip to content

fix(tray): full width at portrait orientation #2547

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Feb 20, 2024
Merged

fix(tray): full width at portrait orientation #2547

merged 4 commits into from
Feb 20, 2024

Conversation

jawinn
Copy link
Collaborator

@jawinn jawinn commented Feb 20, 2024 •
edited
Loading

Description

The Tray was not appearing at the intended 100% width in portrait orientation because of a max-inline-size declaration coming from the .spectrum-Modal class used alongside .spectrum-Tray. This makes sure that the max-inline-size is 100%.

This also:

  • Adds a requested mod for rounding in portrait orientation: --mod-tray-corner-radius-portrait. The landscape rounding can already be changed by --mod-tray-corner-radius.
  • Cleans up some of the docs, including the guidelines link that was previously 404.
  • forced-colors: Replaces an unknown system color with the one that is defined in the current spec.
  • forced-colors: applies border to the correct element, so that rounded corners appear. Previously the border was on a nested element with padding around it, so the borders weren't really to the edge of the Tray.
  • Fixes the heading control in Storybook to be usable.

CSS-698

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

  • Tray appears 100% width at portrait, with square corners. And continues to display the same at landscape (limited width and rounded top corners).
  • Test in the inspector that both --mod-tray-corner-radius-portrait and --mod-tray-corner-radius work. Including forced-colors mode.
  • Proofread docs changes
  • Heading control in Storybook changes the heading text

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.

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 20, 2024 •
edited
Loading

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 20, 2024 •
edited
Loading

File metrics

Summary

Total size: 3.94 MB*
Total change (Δ): ⬆ 0.68 KB (0.02%)
Table reports on changes to a package's main file. Other changes can be found in the collapsed "Details" below.

Package Size Δ
tray 3.78 KB ⬆ 0.21 KB
Details

tray

File Head Base Δ
index-base.css 3.78 KB 3.57 KB ⬆ 0.21 KB (5.83%)
index-vars.css 3.78 KB 3.57 KB ⬆ 0.21 KB (5.83%)
index.css 3.78 KB 3.57 KB ⬆ 0.21 KB (5.83%)
mods.json 0.45 KB 0.41 KB ⬆ 0.04 KB (9.95%)
* 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.

@jawinn jawinn force-pushed the jawinn/css-698-fix-tray-full-width branch from 1f280c2 to 5252c7f Compare February 20, 2024 16:22
@pfulton pfulton self-requested a review February 20, 2024 18:39
@jawinn jawinn force-pushed the jawinn/css-698-fix-tray-full-width branch from 4d79803 to 42d0baa Compare February 20, 2024 19:15
@jawinn @pfulton
docs(tray): docs update and fix guidelines link
6d3fd6b 
Fix incorrect Tray guidelines link that was 404 not found.
Update descriptions in docs to clarify viewport orientation differences.
@jawinn @pfulton
fix(tray): full width at portrait
e016746 
The Tray was not appearing as 100% width in portrait orientation because
of a max-inline-size declaration coming from the .spectrum-Modal class
used with .spectrum-Tray
@jawinn @pfulton
fix(tray): whcm use border on correct element to show rounding
1250095 
Rounded corners were not showing in forced-colors mode, because the
border that was appearing on the nested Dialog element, which has
padding. Moves the high contrast border from that element to the root
Tray element.
@jawinn @pfulton
fix(tray): storybook heading text control and unused import
f8d16da 
The Storybook "heading" control was previously not usable, due to how
the arg was being passed through to the Dialog. This text string control
will now change the heading.

Replaces an "unused" import for Modal whose CSS is still necessary for
the Tray to render correctly. The unused import would be deleted on
save/fix, resulting in the wrong styles. Replaced the template import
with an import of just the CSS for Modal.
@pfulton pfulton force-pushed the jawinn/css-698-fix-tray-full-width branch from 42d0baa to f8d16da Compare February 20, 2024 19:34
@pfulton pfulton added the run_vrt For use on PRs looking to kick off VRT label Feb 20, 2024
@pfulton pfulton merged commit c9a79f2 into main Feb 20, 2024
@pfulton pfulton deleted the jawinn/css-698-fix-tray-full-width branch February 20, 2024 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pfulton pfulton pfulton approved these changes

Assignees
No one assigned
Labels
run_vrt For use on PRs looking to kick off VRT
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jawinn @pfulton