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(splitview)!: migrate tokens #2103

Merged
merged 10 commits into from
Sep 5, 2023
Merged

feat(splitview)!: migrate tokens #2103

merged 10 commits into from
Sep 5, 2023

Conversation

mlogsdon18
Copy link
Contributor

@mlogsdon18 mlogsdon18 commented Aug 18, 2023

Description

This PR is a DIY Migration for the Split view component. This also adds CSS to prevent touch-action on the gripper on mobile.

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.

  • Open the docs site for the Split view component and ensure that the component does not have any visible styling changes @jenndiaz
  • View the docs site in a simulated touch environment (you can do so with Chrome dev tools) and verify that the down event styling still functions correctly @jenndiaz

Regression testing

Validate:

  1. A legacy documentation page (such as accordion), including:
  • The page renders correctly
  • The page is accessible
  • The page is responsive
  1. A migrated documentation page (such as action group), including:
  • The page renders correctly
  • The page is accessible
  • The page is responsive

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

@mlogsdon18 mlogsdon18 force-pushed the css-502-splitview-tokens branch from c541353 to e7aecba Compare August 18, 2023 18:58
@github-actions
Copy link
Contributor

github-actions bot commented Aug 18, 2023

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

@github-actions github-actions bot temporarily deployed to pull request August 18, 2023 19:05 Inactive
@mlogsdon18 mlogsdon18 marked this pull request as draft August 18, 2023 19:07
@github-actions github-actions bot temporarily deployed to pull request August 18, 2023 19:24 Inactive
@mlogsdon18 mlogsdon18 marked this pull request as ready for review August 18, 2023 19:35
@github-actions github-actions bot temporarily deployed to pull request August 18, 2023 19:39 Inactive
@mlogsdon18 mlogsdon18 force-pushed the css-502-splitview-tokens branch from 30d130f to ee4a2c4 Compare August 21, 2023 15:32
@github-actions github-actions bot temporarily deployed to pull request August 21, 2023 15:42 Inactive
@mlogsdon18 mlogsdon18 force-pushed the css-502-splitview-tokens branch 2 times, most recently from 041ce74 to 6acd76c Compare August 22, 2023 20:43
@github-actions github-actions bot temporarily deployed to pull request August 22, 2023 20:52 Inactive
@mlogsdon18 mlogsdon18 force-pushed the css-502-splitview-tokens branch from 6acd76c to 3d7a8bc Compare August 23, 2023 17:27
@github-actions github-actions bot temporarily deployed to pull request August 23, 2023 17:32 Inactive
Copy link
Contributor

@jenndiaz jenndiaz left a comment

Choose a reason for hiding this comment

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

Looks great!

Definitely a WHCM improvement.


&:focus-visible {
outline: none;
background-color: var(--highcontrast-splitview-handle-background-color-focus, var(--mod-splitview-handle-background-color-focus, var(--spectrum-splitview-handle-background-color-focus)));
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to display the focus either in the docs site or in storybook?

Copy link
Collaborator

Choose a reason for hiding this comment

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

A separate story in Storybook could be nice if that's possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about that. The issue is that it's all divs so I can't trigger focus-visible without adding a class. I'm assuming that this focus is used in SWC? The CSS was set up like this but not sure how it works with no focusable elements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh interesting ok the splitter in SWC has a tabindex of 0 so that's why this works there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to have to do an interaction here to get the focus to show up without a class so this will have to wait until the Popover PR goes in... if it ever does

@mlogsdon18 mlogsdon18 force-pushed the css-502-splitview-tokens branch 3 times, most recently from ae07ee9 to 11272ee Compare August 29, 2023 13:45
@mlogsdon18 mlogsdon18 added the run_vrt For use on PRs looking to kick off VRT label Aug 29, 2023
@mlogsdon18 mlogsdon18 force-pushed the css-502-splitview-tokens branch 3 times, most recently from 6030db6 to c2efa6b Compare August 29, 2023 14:52
@github-actions github-actions bot temporarily deployed to pull request August 29, 2023 15:41 Inactive
@github-actions github-actions bot removed the run_vrt For use on PRs looking to kick off VRT label Aug 29, 2023
@mlogsdon18 mlogsdon18 added the run_vrt For use on PRs looking to kick off VRT label Aug 29, 2023
@github-actions github-actions bot temporarily deployed to pull request August 29, 2023 18:28 Inactive
@github-actions github-actions bot removed the run_vrt For use on PRs looking to kick off VRT label Aug 29, 2023
@mlogsdon18 mlogsdon18 force-pushed the css-502-splitview-tokens branch from ba3dc22 to 1ef04a7 Compare August 29, 2023 19:00
@github-actions github-actions bot temporarily deployed to pull request August 29, 2023 19:08 Inactive
@mlogsdon18 mlogsdon18 added the run_vrt For use on PRs looking to kick off VRT label Aug 29, 2023
@github-actions github-actions bot removed the run_vrt For use on PRs looking to kick off VRT label Aug 29, 2023
@github-actions github-actions bot temporarily deployed to pull request August 30, 2023 12:57 Inactive
@mlogsdon18 mlogsdon18 force-pushed the css-502-splitview-tokens branch from 5191573 to 144144f Compare September 1, 2023 13:38
@github-actions github-actions bot temporarily deployed to pull request September 1, 2023 13:46 Inactive
@mlogsdon18 mlogsdon18 force-pushed the css-502-splitview-tokens branch from 144144f to 46ca320 Compare September 5, 2023 13:40
@github-actions github-actions bot temporarily deployed to pull request September 5, 2023 13:46 Inactive
@pfulton pfulton added run_vrt For use on PRs looking to kick off VRT and removed run_vrt For use on PRs looking to kick off VRT labels Sep 5, 2023
@pfulton pfulton force-pushed the css-502-splitview-tokens branch from 46ca320 to e28a626 Compare September 5, 2023 16:47
@github-actions github-actions bot temporarily deployed to pull request September 5, 2023 17:05 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 5, 2023 17:55 Inactive
@pfulton pfulton added the run_vrt For use on PRs looking to kick off VRT label Sep 5, 2023
@github-actions github-actions bot removed the run_vrt For use on PRs looking to kick off VRT label Sep 5, 2023
@pfulton pfulton merged commit 4f39c5d into main Sep 5, 2023
@pfulton pfulton deleted the css-502-splitview-tokens branch September 5, 2023 18:55
Rajdeepc pushed a commit that referenced this pull request Sep 11, 2023
BREAKING CHANGE: migrates SplitView to use `@adobe/spectrum-tokens`

Additionally:
* fix(splitview): remove touch-action from gripper
* refactor(splitview): combine skin.css with index.css and delete skin.css
* feat(splitview)!: updating to use core tokens
* chore(splitview): update mods
* style(splitview): add whcm styling
* chore(splitview): fix linter errors

use two colon pseudo elements
fix max nesting depth

* chore(splitview): working on adding focus story
* chore(splitview): add storybook interaction add-on
* chore(splitview): use latest version of tokens
* fix(splitview): use vertical gripper width for vertical gripper

---------

Co-authored-by: Patrick Fulton <[email protected]>
This was referenced Apr 26, 2024
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.

3 participants