Skip to content

fix(splitview): add missing is-focused class#2932

Merged
marissahuysentruyt merged 1 commit intospectrum-twofrom
marissahuysentruyt/css-856-splitview-focus-class
Jul 24, 2025
Merged

fix(splitview): add missing is-focused class#2932
marissahuysentruyt merged 1 commit intospectrum-twofrom
marissahuysentruyt/css-856-splitview-focus-class

Conversation

@marissahuysentruyt
Copy link
Copy Markdown
Collaborator

@marissahuysentruyt marissahuysentruyt commented Jul 25, 2024

Description

Several eons ago, we noticed that the argTypes for the Split view template needed to be adjusted to showcase the Horizontally Focused story appropriately. In a similar vein, I noticed that our active styles for the collapsible and resizable variants weren't captured in the testing grid. This PR updates all of that.

Jira/Specs

CSS-856

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

  • Pull down the branch to run locally or visit the deploy preview. (@cdransf)
  • Visit the split view page. (@cdransf)
  • On the docs page, the "horizontally focused" story is blue, indicating the is-focus-visible class/state on the .spectrum-SplitView-splitter .is-draggable element. (@cdransf)
  • Visit the testing grid. (@cdransf)
  • Verify that each test case includes the gripper, except for the default split view. All resizable and collapsible split views should have a gripper. (@cdransf)
  • Verify that the resizable+active test cases render gray-800 for the splitter and the gripper elements. (@cdransf)
  • Verify that the resizable+focused test cases render focus-indicator-color for the splitter and the gripper elements. (@cdransf)

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 documentation, I have updated the documentation accordingly.
  • ✨ This pull request is ready to merge. ✨

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jul 25, 2024

⚠️ No Changeset found

Latest commit: 9410aa0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 25, 2024

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 25, 2024

File metrics

Summary

Total size: 1.43 MB*

🎉 No changes detected in any packages

* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-856-splitview-focus-class branch from 4cb17f5 to b2c42c4 Compare July 29, 2024 18:52
@marissahuysentruyt marissahuysentruyt added wip This is a work in progress, don't judge. do not merge A flag for a branch indicating it should not be merged. labels Jul 30, 2024
@marissahuysentruyt marissahuysentruyt self-assigned this Feb 17, 2025
@castastrophe castastrophe force-pushed the main branch 11 times, most recently from c68f4e3 to d2272ea Compare March 12, 2025 21:56
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-856-splitview-focus-class branch from b2c42c4 to dbecd0b Compare July 21, 2025 16:27
@marissahuysentruyt marissahuysentruyt changed the base branch from main to spectrum-two July 21, 2025 16:28
{
testHeading: "Focused",
isFocused: true,
testHeading: "Resizable/collapsible",
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Resizable and collapsible are technically two different things in SWC, where collapsible can in fact collapse all of one of the panes and resizable maintains to min-width for the panes. However, the collapsible variant will also need the gripper (which is only added in isResizable splitviews right now.)

Copy link
Copy Markdown
Collaborator Author

@marissahuysentruyt marissahuysentruyt Jul 22, 2025

Choose a reason for hiding this comment

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

We could definitely come back to make sure the active WHCM color is what we want, and clean up the args for this component (i.e. isCollapsible should also add the gripper, not just isResizable). I didn't want to get into too much of that with this PR quite yet, but I can make a ticket sort of like an "S2 migration."

[`${rootClass}-splitter`]: true,
["is-draggable"]: isResizable,
"is-focused": isFocused,
["is-active"]: isActive,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have styles for the active gripper, so adding the isActive arg here to capture in the tests.

Copy link
Copy Markdown
Contributor

@castastrophe castastrophe left a comment

Choose a reason for hiding this comment

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

This is a great update that improves test coverage. I'm cool if you want to move this forward.

@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-856-splitview-focus-class branch from dbecd0b to 0b8e719 Compare July 22, 2025 19:57
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 22, 2025

📚 Branch preview

PR #2932 has been deployed to Azure Blob Storage: https://spectrumcss.z13.web.core.windows.net/pr-2932/index.html.

@marissahuysentruyt marissahuysentruyt added size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. S2 Spectrum 2 skip_vrt Add to a PR to skip running VRT (but still pass the action) and removed wip This is a work in progress, don't judge. do not merge A flag for a branch indicating it should not be merged. labels Jul 22, 2025
@marissahuysentruyt marissahuysentruyt marked this pull request as ready for review July 22, 2025 21:03
- fixes orientation of top & bottom test cases to be vertical
- adds isActive to template for testing and adds isActive to test cases
to accurately update baselines for VRTs
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-856-splitview-focus-class branch from 0b8e719 to 9410aa0 Compare July 24, 2025 12:23
@marissahuysentruyt marissahuysentruyt merged commit 9410aa0 into spectrum-two Jul 24, 2025
13 checks passed
@marissahuysentruyt marissahuysentruyt deleted the marissahuysentruyt/css-856-splitview-focus-class branch July 24, 2025 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review S2 Spectrum 2 size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. skip_vrt Add to a PR to skip running VRT (but still pass the action)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants