Skip to content

chore(split-view): author migration plan - #6808

Closed
cdransf wants to merge 5 commits into
mainfrom
cdransf/s2-migration-split-view-plan
Closed

cdransf wants to merge 5 commits into
mainfrom
cdransf/s2-migration-split-view-plan

Conversation

@cdransf

@cdransf cdransf commented Sep 25, 2026

Copy link
Copy Markdown
Member

Description

Adds the Phase 1 migration-prep plan for split-view: 1st-gen API surface, dependencies, breaking-change/additive classification, gen2 API decisions, core/SWC architecture split, migration checklist, and open questions. Resolves several architecture and naming questions using precedent from color-handle and progress-circle, and folds settled decisions into a decision log.

Motivation and context

Phase 1 of the 1st-gen → gen2 migration for split-view (SWC-2265, epic SWC-2263). This plan must be reviewed before any implementation work begins.

Related issue(s)

  • fixes SWC-2265

Screenshots (if appropriate)

N/A — planning document, no visual/code changes.

Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes. — N/A, no code in this PR
  • I have included a well-written changeset if my change needs to be published. — N/A, docs only, nothing published
  • I have included updated documentation if my change required it.

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

  • Confirm plan accuracy
    1. Read CONTRIBUTOR-DOCS/03_project-planning/03_components/split-view/migration-plan.md
    2. Cross-check the API surface and breaking-change tables against 1st-gen/packages/split-view/src/SplitView.ts
    3. Confirm the open questions and decision log entries are correctly scoped

Device review

N/A — planning document, no rendered UI.

Accessibility testing checklist

N/A — this PR adds a planning document only; it does not change any component code or markup. Accessibility recommendations from the existing accessibility-migration-analysis.md are referenced and folded into this plan's must-ship items (B5–B9) for the implementation phase, where keyboard and screen reader testing will apply.

@cdransf cdransf self-assigned this Sep 25, 2026
@cdransf cdransf added Status:Ready for review PR ready for review or re-review. Component:Split view Spectrum 2 Issues related to Spectrum 2 labels Sep 25, 2026
@changeset-bot

changeset-bot Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 3cee818

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.

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

📚 Branch Preview Links

🔍 Gen1 Visual Regression Test Results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

Deployed to Azure Blob Storage: pr-6808

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch 3 times, most recently from c3ac140 to c5b5562 Compare September 25, 2026 22:42
@cdransf
cdransf marked this pull request as ready for review September 25, 2026 23:17
@cdransf
cdransf requested a review from a team as a code owner September 25, 2026 23:17
@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch from c5b5562 to 424b1ce Compare September 25, 2026 23:17
@cdransf cdransf added the skip_vrt Skip VRT build; mark UI Tests green without running Chromatic label Sep 28, 2026
@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch from 424b1ce to 809afe1 Compare September 28, 2026 22:35
@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch 3 times, most recently from 91619e0 to f30fb2b Compare September 30, 2026 23:30

@5t3ph 5t3ph left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great start on this!

Comment thread CONTRIBUTOR-DOCS/03_project-planning/03_components/split-view/migration-plan.md Outdated
Comment thread CONTRIBUTOR-DOCS/03_project-planning/03_components/split-view/migration-plan.md Outdated
Comment thread CONTRIBUTOR-DOCS/03_project-planning/03_components/split-view/migration-plan.md Outdated
Comment thread CONTRIBUTOR-DOCS/03_project-planning/03_components/split-view/migration-plan.md Outdated
Comment thread CONTRIBUTOR-DOCS/03_project-planning/03_components/split-view/migration-plan.md Outdated
@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch from f30fb2b to ffeedea Compare October 1, 2026 18:45
@cdransf
cdransf requested a review from 5t3ph October 1, 2026 19:31
@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch from b12c4c1 to 260e3cf Compare October 1, 2026 19:31
@cdransf cdransf added Status:Ready for re-review PR has had its feedback addressed and is once again ready for review. and removed Status:Ready for review PR ready for review or re-review. labels Oct 1, 2026

@5t3ph 5t3ph left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! :shipit:

@Rajdeepc Rajdeepc left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The API updates and migration direction look sensible. My recommendation is to clarify the non-drag pointer alternative, named-slot contract, and listener lifecycle before treating the plan as implementation-ready. I've left a few focused suggestions inline.

Comment thread CONTRIBUTOR-DOCS/03_project-planning/03_components/split-view/migration-plan.md Outdated
@cdransf
cdransf requested a review from Rajdeepc October 5, 2026 16:00
@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch from 485a0df to 91b8b6b Compare October 5, 2026 20:09
Comment thread CONTRIBUTOR-DOCS/03_project-planning/03_components/split-view/migration-plan.md Outdated
@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch from 8686d3f to ea3c69b Compare October 6, 2026 15:38
@cdransf
cdransf requested a review from Rajdeepc October 6, 2026 15:40
@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch from ea3c69b to 6e469ef Compare October 6, 2026 17:35
- rename label to accessible-label and vertical to orientation to match
existing gen2 apis
- move panes to named primary and secondary slots so assignment is
explicit and render can template each pane
- resolve aria-orientation as line orientation per aria separator role
and apg window splitter pattern
- add shift plus arrow double-step resize, shipping with the migration
- unlink jira references and fix broken in-page anchors
@cdransf
cdransf force-pushed the cdransf/s2-migration-split-view-plan branch from 6e469ef to 3cee818 Compare October 6, 2026 20:14

| Event | Detail | Fires when |
| ----- | ------ | ----------- |
| `change` | none (plain, bubbling, composed `Event`) | The splitter position changes via pointer drag or keyboard, including collapse to an extreme. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i didnt see an area below to capture this but want to make sure we are prepending swc- to all events we send

| **B5** | Add `aria-valuemin="0"` / `aria-valuemax="100"` alongside the existing `aria-valuenow` whenever `resizable`. | `aria-valuenow` only. | `aria-valuemin`/`aria-valuemax` always paired with `aria-valuenow`. | None; additive attribute, not observable as an API change. |
| **B6** | Keep the 1st-gen `aria-orientation` mapping: it describes the divider **line's** orientation, not the axis of motion, per the [ARIA separator role](https://www.w3.org/TR/wai-aria-1.2/#separator) and the [APG window splitter pattern](https://www.w3.org/WAI/ARIA/apg/patterns/windowsplitter/). Verify in the screen reader pass. | Sets `aria-orientation` to describe the divider **line's** visual orientation. | Same. | None. |
| **B7** | Move `aria-controls` off a plain ID string crossing the shadow boundary onto the project's element-reference IDL pattern (`ariaControlsElements`), per the accessibility migration analysis, reusing the shape from `Popover.base.ts`. Scope (primary pane only vs. both) remains open, see `Q4`. | `aria-controls="<id>"` referencing a light-DOM child's `id`, assigned by the component itself. | Element-reference IDL property in addition to (or instead of) the ID string. | None for consumers using the public attribute/property surface; internal wiring change only. |
| **B8** | Preserve the SWC-276 fix: default `aria-label` ("Resize the panels") whenever `resizable` and no `accessible-label` is set. Never ship a focusable, unnamed divider. | Fixed in 1st-gen. | Same behavior, unit-tested as a regression guard. | None. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should also include a warning if an accessible label is expected when a resizable is present. This will be needed for the internationalization by consumers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rule of thumb for any default string we provide should have a warning communicating how to properly set it

| `orientation` | `'horizontal' \| 'vertical'` | `'horizontal'` | `orientation`, reflected | **Confirmed.** Replaces 1st-gen `vertical`, see `B10`. `vertical` stacks panes top/bottom. |
| `resizable` | `boolean` | `false` | `resizable`, reflected | **Confirmed.** Unchanged from 1st-gen. |
| `collapsible` | `boolean` | `false` | `collapsible`, reflected | **Confirmed.** Unchanged from 1st-gen; still requires `resizable`. |
| `primaryMin` / `primaryMax` | `number` | `0` / `3840` | `primary-min` / `primary-max` | **Confirmed.** Unchanged from 1st-gen. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should these also allow percentages if primarySize allows it? also where is the 3840 coming from?

@cdransf

cdransf commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Closing for now! We'll pick this back up down the road.

@cdransf cdransf closed this Oct 7, 2026
@caseyisonit

Copy link
Copy Markdown
Contributor

ok so I want to request an API change to this. collapsible is too conditional of an API, requires resizable, but dead codes the 4 min/max API's in the same breath.

I believe by default that resizable should satisfy collapsible behavior, then if the min/max API values are set, it becomes more static/controlled. This makes the API scalable and less coupled to nuanced configuration.

Effectively we should remove collapsible and the logic should be based on the other API values as they are added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component:Split view skip_vrt Skip VRT build; mark UI Tests green without running Chromatic Spectrum 2 Issues related to Spectrum 2 Status:Ready for re-review PR has had its feedback addressed and is once again ready for review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants