Skip to content

feat(tabs): 2nd-gen migration plan#6179

Merged
Rajdeepc merged 14 commits intomainfrom
rajdeepc/docs-tabs-migration-plan
Apr 28, 2026
Merged

feat(tabs): 2nd-gen migration plan#6179
Rajdeepc merged 14 commits intomainfrom
rajdeepc/docs-tabs-migration-plan

Conversation

@Rajdeepc
Copy link
Copy Markdown
Contributor

@Rajdeepc Rajdeepc commented Apr 15, 2026

Description

Provides the migration plan for Tabs, per washing machine guidelines.

NOTE: Awaiting official a11y plan (SWC-1898) before this can be finalised and merged.

Related issue(s)

SWC-1899

Reviewer's checklist

Ensure all expected content is included in the migration plan
Help answer any outstanding questions, especially blockers
Help validate/provide feedback for scope, API, and changes/additions

- Create rendering-and-styling migration analysis with full API surface,
  DOM comparison, CSS-to-SWC mapping, and implementation gap summary
- Create migration plan covering all 8 washing machine phases with
  detailed checklists, dependency analysis, breaking changes, testing
  plan (red/green TDD), and open questions
- Create accessibility migration analysis with WCAG 2.2 AA targets,
  APG tabs pattern alignment, automatic vs manual activation, and
  FocusgroupNavigationController integration plan
- Create consumer migration guide (migration.md) with installation,
  quick reference table, breaking changes with before/after examples,
  preserved API documentation, usage examples, and accessibility notes
- Update contributor docs navigation (breadcrumbs, TOC, component index)

Made-with: Cursor
@Rajdeepc Rajdeepc self-assigned this Apr 15, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 15, 2026

⚠️ No Changeset found

Latest commit: ad7d52a

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 Apr 15, 2026

📚 Branch Preview Links

🔍 First Generation 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-6179

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.

@Rajdeepc Rajdeepc changed the title docs(tabs): add migration plan, analysis, and consumer guide feat(tabs): 2nd-gen migration plan Apr 16, 2026
@Rajdeepc Rajdeepc marked this pull request as ready for review April 16, 2026 06:42
@Rajdeepc Rajdeepc requested a review from a team as a code owner April 16, 2026 06:42
@Rajdeepc Rajdeepc added Status:WIP PR is a work in progress or draft Component:Tabs 2nd gen These issues or PRs map to our 2nd generation work to modernizing infrastructure. labels Apr 16, 2026
- `slot="tab-panel"` set programmatically in `firstUpdated`
- Auto-generated `id`: `sp-tab-panel-${randomID()}`

### sp-tabs-overflow (`TabsOverflow`)
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.

Can we confirm if this should be carried over? It seems like when there is not enough space they won't overflow, instead they turn into a Picker.

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.

Copy link
Copy Markdown
Collaborator

@pfulton pfulton Apr 16, 2026

Choose a reason for hiding this comment

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

We also have this behavior in CSS: https://64762974a45b8bc5ca1705a2-yypcfpggii.chromatic.com/?path=/story/components-tabs--default&args=orientation:overflow & this is also referenced in the S2 docs site (you can get to the page via the Figma file)

Copy link
Copy Markdown
Contributor Author

@Rajdeepc Rajdeepc Apr 19, 2026

Choose a reason for hiding this comment

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

@pfulton @rubencarvalho @nikkimk — My recommendation: do not port sp-tabs-overflow in phase 1.

Rationale:

  • S2 replaces scroll-based overflow with Picker-based collapse — these are fundamentally different patterns. Porting the scroll version only to replace it is wasted effort.
  • TabsOverflow has a deep dependency chain (sp-action-button, chevron icons, scrollTabs/scrollState API) that isn't needed for the Picker pattern.
  • It doesn't block core tabs migration — swc-tabs, swc-tab, swc-tab-panel stand alone.

Proposed phasing:

  • Phase 1 (this epic): Core tabs only. No overflow component.
  • Phase 2 (follow-up): Picker-based collapse built into swc-tabs, once design alignment and swc-picker are ready.

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'm OK with this!

| Property | Type | Default | Attribute | Notes |
|---|---|---|---|---|
| `auto` | `boolean` | `false` | `auto` | Automatic activation — selection follows focus. |
| `compact` | `boolean` | `false` | `compact` | Reflected. |
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 this rename to density per Figma/React API?

Copy link
Copy Markdown
Contributor Author

@Rajdeepc Rajdeepc Apr 19, 2026

Choose a reason for hiding this comment

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

Updated

Renamed to density with values 'compact' | 'regular' in the 2nd-gen API table. Added as:

  • new breaking change documenting the migration path (compactdensity="compact")

Migration checklist and styling section also updated to reference density instead of compact.

| `auto` | `boolean` | `false` | `auto` | Automatic activation — selection follows focus. |
| `compact` | `boolean` | `false` | `compact` | Reflected. |
| `direction` | `'horizontal' \| 'vertical'` | `'horizontal'` | `direction` | Reflected. `vertical-right` dropped unless Q4 resolves to keep. |
| `emphasized` | `boolean` | `false` | `emphasized` | Reflected. |
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 don't see emphasized, quiet, or size as still in the S2 API from any sources.

Copy link
Copy Markdown
Contributor Author

@Rajdeepc Rajdeepc Apr 19, 2026

Choose a reason for hiding this comment

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

Great catch

Marked emphasized, quiet, and size with strikethrough in the 2nd-gen API table as "Not in S2 API", pending design confirmation.
Added as:

  • new breaking changes for each removed property

Also updated the TL;DR, migration checklist, styling checklist, Storybook stories list, and VRT plan to reflect potential removal. Q8 (default size) now cross-references Q18 as well.


| Property | Type | Default | Attribute | Notes |
|---|---|---|---|---|
| `auto` | `boolean` | `false` | `auto` | Automatic activation — selection follows focus. |
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.

Rename to keyboardActivation following React and to make more explicit what the impact is?

Copy link
Copy Markdown
Contributor Author

@Rajdeepc Rajdeepc Apr 19, 2026

Choose a reason for hiding this comment

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

Good call — updated

Renamed to keyboardActivation with values 'automatic' | 'manual' (attribute: keyboard-activation) in the 2nd-gen API table. Added as:

  • new breaking change documenting the migration path (autokeyboard-activation="automatic")

Also updated the behavioral semantics section, unit test table, and manual verification matrix to use the new naming.

- **RTL:** Arrow keys swap automatically for `dir="rtl"`.
- **Disabled tabs:** `aria-disabled` tabs remain focusable via arrow keys but are not activatable (Enter/Space/click handlers must be guarded). Natively `disabled` elements are skipped.

### ARIA and keyboard contract
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.

Most of this will be in @nikkimk 's a11y analysis, which will set next to this doc, so we may want to ultimately remove to prevent duplication or conflicting information.

Copy link
Copy Markdown
Contributor Author

@Rajdeepc Rajdeepc Apr 19, 2026

Choose a reason for hiding this comment

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

Agreed — I have trimmed this section and have replaced the full ARIA/keyboard contract section with a concise summary that references the accessibility migration analysis doc.
The migration plan now keeps only:

  • A reference callout to the a11y analysis doc
  • Key ARIA requirements (bullet list summary)
  • 1st-gen bugs table (since those map directly to breaking changes B5–B8)

The detailed WCAG guidelines, ARIA roles/states table, keyboard mapping table, and accessibility tree expectations table have been removed from this doc to avoid duplication with @nikkimk's analysis.


<!-- Document title (editable) -->

# `sp-tabs` Migration Plan
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.

Small change per some Slack discussion this week:

Suggested change
# `sp-tabs` Migration Plan
# Tabs Migration Plan

Copy link
Copy Markdown
Contributor Author

@Rajdeepc Rajdeepc Apr 19, 2026

Choose a reason for hiding this comment

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

Applied — Title updated to # Tabs migration plan (also using sentence case per our text formatting conventions). Breadcrumb updated to match.

**File layout (SWC):**

```
2nd-gen/packages/swc/components/tabs/
Copy link
Copy Markdown
Contributor

@rubencarvalho rubencarvalho Apr 17, 2026

Choose a reason for hiding this comment

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

This is the first component with children components we're migrating. I am OK with the proposed file layout, but maybe we can double-check with the team this is expected. One question I do have, though is how do we show the documentation and API of each of the components? Will it all be one "Tabs" Storybook page or multiple (per component)?

Copy link
Copy Markdown
Contributor Author

@Rajdeepc Rajdeepc Apr 19, 2026

Choose a reason for hiding this comment

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

Good question — added in the open questions section

Since Tabs is the first multi-element component we're migrating, we need team input on whether all child component APIs (swc-tab, swc-tab-panel) should be documented on a single "Tabs" Storybook page, or whether each gets its own page. This decision will also set the precedent for future multi-element components.

For the file layout itself, I think the proposed flat structure under 2nd-gen/packages/swc/components/tabs/ works well, but happy to revisit if the team prefers a different approach.

- Rename title to "Tabs migration plan" per team convention (5t3ph)
- Update 2nd-gen API: `auto` → `keyboardActivation`, `compact` → `density`
  to align with Figma/React Spectrum naming (5t3ph)
- Mark `emphasized`, `quiet`, `size` as not in S2 API, pending design
  confirmation for removal (5t3ph)
- Add S2 Picker-based overflow collapse note to sp-tabs-overflow section
  and new breaking change B26 (rubencarvalho, pfulton)
- Trim ARIA/keyboard contract section to reference a11y analysis doc,
  avoiding duplication with nikkimk's analysis (5t3ph)
- Add open questions Q16–Q20 for API renames, S2 removals, overflow
  pattern, and Storybook multi-component doc structure (rubencarvalho)
- Add breaking changes B21–B26 for all newly identified API changes
- Update migration checklist, test tables, and VRT plan to reflect
  renamed/removed properties

Made-with: Cursor
@Rajdeepc Rajdeepc added Status:Ready for review PR ready for review or re-review. and removed Status:WIP PR is a work in progress or draft labels Apr 19, 2026
S2 replaces scroll-based overflow with Picker-based collapse, making
a scroll-based port throwaway work. Resolve Q5, Q14, Q19 as deferred
and update TL;DR, B26, and blocking questions list accordingly.

Made-with: Cursor
Copy link
Copy Markdown
Contributor

@rubencarvalho rubencarvalho 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 in a good spot, I have some comments but they are not blocking! I leave it up to you if you want to address those now 😄


| # | Item | Blocking? | Status | Owner |
|---|---|---|---|---|
| **Q1** | 2nd-gen keyboard controller readiness — is [#6129](https://github.com/adobe/spectrum-web-components/pull/6129) merged? | Yes | Open | #6129 author |
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.

This is merged! 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep — Q1 is now marked Resolved in the plan. TL;DR and behavioral semantics updated to reference FocusgroupNavigationController by name. Tabs will use this controller directly, no new primitives.

| # | Item | Blocking? | Status | Owner |
|---|---|---|---|---|
| **Q1** | 2nd-gen keyboard controller readiness — is [#6129](https://github.com/adobe/spectrum-web-components/pull/6129) merged? | Yes | Open | #6129 author |
| **Q2** | `Focusable` mixin disposition — is it needed in 2nd-gen tabs, or should focus be managed entirely via the 2nd-gen keyboard controller? | Yes | Open | Architecture reviewer |
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.

Can we answer this already or not?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes — Q2 is now Resolved. With FocusgroupNavigationController merged, the Focusable mixin is not needed for tabs. The controller handles roving tabindex for the tab collection, and delegatesFocus on the shadow root replaces what Focusable provided for individual focus delegation. focusElement getter is dropped (B10).

| **Q1** | 2nd-gen keyboard controller readiness — is [#6129](https://github.com/adobe/spectrum-web-components/pull/6129) merged? | Yes | Open | #6129 author |
| **Q2** | `Focusable` mixin disposition — is it needed in 2nd-gen tabs, or should focus be managed entirely via the 2nd-gen keyboard controller? | Yes | Open | Architecture reviewer |
| **Q3** | Cross-root ARIA: how will `aria-controls` / `aria-labelledby` ID references resolve if 2nd-gen changes the DOM arrangement? | Yes | Open | Implementation |
| **Q4** | Should `direction="vertical-right"` be carried forward? Not in Spectrum CSS; may be needed for specific layouts. | No | Open | Design |
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.

What does the Figma / React say?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Checked — neither Figma nor React Spectrum have a vertical-right equivalent. React Spectrum supports orientation="vertical" only (no right-side variant). In 1st-gen SWC, vertical-right just controls which side the selection indicator renders on — consumers can achieve the same result with CSS positioning. Updated Q4 with a recommendation to drop it.

| **Q2** | `Focusable` mixin disposition — is it needed in 2nd-gen tabs, or should focus be managed entirely via the 2nd-gen keyboard controller? | Yes | Open | Architecture reviewer |
| **Q3** | Cross-root ARIA: how will `aria-controls` / `aria-labelledby` ID references resolve if 2nd-gen changes the DOM arrangement? | Yes | Open | Implementation |
| **Q4** | Should `direction="vertical-right"` be carried forward? Not in Spectrum CSS; may be needed for specific layouts. | No | Open | Design |
| **Q6** | Should `enableTabsScroll` be renamed to a simpler attribute (e.g., `scroll`)? | No | Open | API reviewer |
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.

This won't exist in this phase, so maybe we can defer?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed — Q6 is now Resolved and deferred. enableTabsScroll is part of the scroll-based overflow behavior which is entirely phase 2 scope (Q5, Q19). Will revisit alongside the Picker-based collapse design.

| **Q3** | Cross-root ARIA: how will `aria-controls` / `aria-labelledby` ID references resolve if 2nd-gen changes the DOM arrangement? | Yes | Open | Implementation |
| **Q4** | Should `direction="vertical-right"` be carried forward? Not in Spectrum CSS; may be needed for specific layouts. | No | Open | Design |
| **Q6** | Should `enableTabsScroll` be renamed to a simpler attribute (e.g., `scroll`)? | No | Open | API reviewer |
| **Q7** | Event naming: keep `change` as-is or rename to `swc-change`? Renaming would silently break every consumer using `@change` or `addEventListener('change', ...)`. Strongly recommend keeping `change`. | **Yes** | Open | API reviewer |
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.

Can we align on this? It's actually weird, we sometimes append swc- other times not 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agree it needs alignment. Per the JSDoc standards guide, the convention is: use DOM-style names where applicable (change, input, close) and prefix with swc- only when namespacing is needed to avoid conflicts. change matches native <select> semantics here. The swc- prefix is for custom events with no native equivalent (e.g., swc-focusgroup-navigation-active-change from #6129). Updated Q7 with this rationale and flagged it for a sync discussion since it sets precedent.

| **Q4** | Should `direction="vertical-right"` be carried forward? Not in Spectrum CSS; may be needed for specific layouts. | No | Open | Design |
| **Q6** | Should `enableTabsScroll` be renamed to a simpler attribute (e.g., `scroll`)? | No | Open | API reviewer |
| **Q7** | Event naming: keep `change` as-is or rename to `swc-change`? Renaming would silently break every consumer using `@change` or `addEventListener('change', ...)`. Strongly recommend keeping `change`. | **Yes** | Open | API reviewer |
| **Q8** | Default size: should `size="m"` be set by default, or keep `noDefaultSize`? Spectrum 2 defaults to M. **Note:** 5t3ph reports `size` is not in the S2 API at all (see Q18). | No | Open | Design |
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.

If no size, can we get rid of it altogether? :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — Q8 is now Resolved as moot. Since size is not in the S2 API (Q18/B25), there is no sizing property and therefore no default size question. If Q18 is somehow reversed and size is restored, we reopen Q8.

Copy link
Copy Markdown
Contributor

@caseyisonit caseyisonit left a comment

Choose a reason for hiding this comment

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

Can you update this plan with the approved focus management guidance? i see references to the PR but thats been merged in and might have impacts to the guidance for 2nd-gen. Make sure the tabs are implementing the FocusgroupNavigationController and not coming up with something new.

Also is this the feature branch or is the plan just going in to main? im cool with it but wanted to check if this was meant to point to main.

@Rajdeepc
Copy link
Copy Markdown
Contributor Author

Can you update this plan with the approved focus management guidance? i see references to the PR but thats been merged in and might have impacts to the guidance for 2nd-gen. Make sure the tabs are implementing the FocusgroupNavigationController and not coming up with something new.

Also is this the feature branch or is the plan just going in to main? im cool with it but wanted to check if this was meant to point to main.

I don't see an issue this going into main. But yes there were few gaps with the new focus management approach which i have covered now.

@Rajdeepc Rajdeepc requested a review from caseyisonit April 22, 2026 05:38
Copy link
Copy Markdown
Contributor

@5t3ph 5t3ph left a comment

Choose a reason for hiding this comment

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

Commented on a few of the questions marked both "Blocking" and "Open" but otherwise, I'd be ready to approve.

| **Q3** | Cross-root ARIA: how will `aria-controls` / `aria-labelledby` ID references resolve if 2nd-gen changes the DOM arrangement? | Yes | Open | Implementation |
| **Q4** | Should `direction="vertical-right"` be carried forward? Not in Spectrum CSS or React Spectrum (RS supports `orientation="vertical"` only, no right-side variant). This is a 1st-gen SWC-only addition that controls which side the selection indicator renders on. **Recommendation: drop it.** Consumers can achieve the same layout with CSS (`direction: rtl` or custom positioning). Keeping it creates API surface with no upstream equivalent. | No | Open — leaning drop | Design |
| **Q6** | ~~Should `enableTabsScroll` be renamed to a simpler attribute (e.g., `scroll`)?~~ **Resolved: deferred.** `enableTabsScroll` is part of the scroll-based overflow behavior, which is deferred to phase 2 (Q5, Q19). This question will be revisited alongside the Picker-based collapse design. | No | **Resolved** | — |
| **Q7** | Event naming: keep `change` as-is or rename to `swc-change`? **Recommendation: keep `change`.** Per the [JSDoc standards guide](../../../02_style-guide/02_typescript/07_jsdoc-standards.md), the convention is to use DOM-style names where applicable and prefix with `swc-` only when namespacing is needed to avoid conflicts. `change` matches native `<select>` semantics and renaming would silently break all consumers. The `swc-` prefix is reserved for custom events that have no native equivalent (e.g., `swc-focusgroup-navigation-active-change`). **Needs team alignment — flag for sync discussion.** | **Yes** | Open | API reviewer |
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.

Do you feel the group needs to resolve this to be able to finish this plan?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No — the evidence from S2 sources is clear enough. Downgraded Q18 from blocking. Design confirmation can happen during implementation without holding up the plan.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Design confirmation can happen during implementation.

|---|---|---|---|---|
| **Q1** | ~~2nd-gen keyboard controller readiness — is [#6129](https://github.com/adobe/spectrum-web-components/pull/6129) merged?~~ **Resolved.** `FocusgroupNavigationController` merged into `main` on 2026-04-16. Tabs must use this controller (not invent a new one). It provides roving tabindex, directional navigation (horizontal/vertical/both/grid), RTL support, wrap, skip-disabled, memory, and typeahead. | No | **Resolved** | — |
| **Q2** | ~~`Focusable` mixin disposition — is it needed in 2nd-gen tabs?~~ **Resolved.** Not needed. `FocusgroupNavigationController` (Q1) manages roving tabindex for the tab collection. `swc-tabs` should use `delegatesFocus` on its shadow root instead of the `Focusable` mixin. The `focusElement` getter is dropped (B10). Individual `swc-tab` elements get their focusability from the controller, not from `Focusable`. | No | **Resolved** | — |
| **Q3** | Cross-root ARIA: how will `aria-controls` / `aria-labelledby` ID references resolve if 2nd-gen changes the DOM arrangement? | Yes | Open | Implementation |
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.

@nikkimk do you have input at this time, or would some collab during implementation work?

Comment on lines +769 to +771
| **Q16** | Rename `auto` boolean to `keyboardActivation` (`'automatic' \| 'manual'`) to align with React Spectrum? More descriptive API that clarifies the behavior. | No | Open | API reviewer |
| **Q17** | Rename `compact` boolean to `density` (`'compact' \| 'regular'`) to align with Figma/React Spectrum? | No | Open | API reviewer / Design |
| **Q18** | `emphasized`, `quiet`, and `size` do not appear in the S2 API (per 5t3ph review). Confirm removal. If removed, these become breaking changes (B23, B24, B25) for consumers using them. | **Yes** | Open | Design |
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.

@Rajdeepc these are all considered resolved now, correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct — Q5, Q14, and Q19 are all resolved as deferred to phase 2. The scope/prerequisites table reflects this.

Copy link
Copy Markdown
Contributor Author

@Rajdeepc Rajdeepc Apr 23, 2026

Choose a reason for hiding this comment

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

Confirmed

| **Q9** | Module-level exports (`ScaledIndicator`, scroll helpers) — carry forward as public API, internalize, or drop? | No | Open | API reviewer |
| **Q10** | `selectionIndicatorStyle` / `shouldAnimate` (`attribute: false` properties) — confirm not carried forward. | No | Open | Implementation |
| **Q11** | `<label>` element inside `sp-tab` shadow DOM — consumers targeting via CSS may break when removed. Document as internal DOM change. | No | Open | Migration guide author |
| **Q12** | `--highcontrast-tabs-*` tokens — verify whether Spectrum 2 handles automatically or needs explicit migration. | No | Open | CSS reviewer |
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.

They may need explicit attention, but follow the guidelines of only adding if we actually notice a problem. Since Tabs is an invented control, this likely is an instance where we will need to apply some overrides.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good guidance. Updated Q12 to reference the forced-colors requirements doc and noted that since Tabs is an invented control, we will likely need some overrides. Will verify during implementation rather than speculating now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated with link to forced-colors requirements doc. Noted Tabs will likely need overrides.

| **Q17** | Rename `compact` boolean to `density` (`'compact' \| 'regular'`) to align with Figma/React Spectrum? | No | Open | API reviewer / Design |
| **Q18** | `emphasized`, `quiet`, and `size` do not appear in the S2 API (per 5t3ph review). Confirm removal. If removed, these become breaking changes (B23, B24, B25) for consumers using them. | **Yes** | Open | Design |

### Scope and prerequisites
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.

These are all marked non-blocking, but are there any decisions that actually could/should be made at this stage? Or are they all easier to work out in implementation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed them all. Q9 (module exports) and Q10 (indicator animation internals) are best resolved during implementation — no consumer-facing decision needed upfront. Q11 resolved per your guidance below. Q13 (test gaps) is a checklist for the test author, not a decision. Q15 (slot auto-assignment) is the only one that could be decided now, but it depends on how the 2nd-gen DOM is structured, so deferring to implementation is pragmatic. Updated each item with disposition notes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added disposition notes to Q9 and Q10 — both resolve during implementation, not upfront decisions.

| **Q5** | ~~Should `sp-tabs-overflow` be migrated in this epic or deferred?~~ **Resolved: deferred to phase 2.** S2 Picker-based collapse requires design alignment and `swc-picker` availability. Porting scroll-based overflow would be throwaway work. | No | **Resolved** | — |
| **Q9** | Module-level exports (`ScaledIndicator`, scroll helpers) — carry forward as public API, internalize, or drop? | No | Open | API reviewer |
| **Q10** | `selectionIndicatorStyle` / `shouldAnimate` (`attribute: false` properties) — confirm not carried forward. | No | Open | Implementation |
| **Q11** | `<label>` element inside `sp-tab` shadow DOM — consumers targeting via CSS may break when removed. Document as internal DOM change. | No | Open | Migration guide author |
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.

"internal DOM changes" should not be a concern of consumers, so IMO they don't belong on consumer migration guides.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed — Q11 resolved. Shadow DOM internals are not a consumer API contract and do not belong in migration guides. Removed from the open items.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Resolved. Removed from open items.

Copy link
Copy Markdown
Contributor

@5t3ph 5t3ph left a comment

Choose a reason for hiding this comment

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

LGTM!

- `direction="vertical-right"` is a 1st-gen SWC addition not in Spectrum CSS; removal is a breaking change for consumers using it (Q4)
- S2 overflow changes from scroll-based to Picker-based collapse — **deferred to phase 2** (Q5, Q19 resolved)
- Several public API surfaces are removed: `rovingTabindexController` field, `focusElement` getter (`Focusable` dropped — Q2 resolved), module-level exports, CSS deep imports
- `change` event rename to `swc-change` would silently break all consumers — strongly recommend keeping `change`
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 standardize our event naming and would push for this change, the migration to this component is already a breaking change. as long as this is tracking the consumer migration plan we should be safe to improve this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@caseyisonit Can you please add this as a part of our sync discussion.


| # | Item | Blocking? | Status | Owner |
|---|---|---|---|---|
| **Q1** | ~~2nd-gen keyboard controller readiness — is [#6129](https://github.com/adobe/spectrum-web-components/pull/6129) merged?~~ **Resolved.** `FocusgroupNavigationController` merged into `main` on 2026-04-16. Tabs must use this controller (not invent a new one). It provides roving tabindex, directional navigation (horizontal/vertical/both/grid), RTL support, wrap, skip-disabled, memory, and typeahead. | No | **Resolved** | — |
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.

Let us move all resolved to the respective sections as decisions? Reduces the noise here and makes it easier to see what's still "open".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Restructured the section into three parts:

  • Open — architecture and behavior (Q3, Q4, Q7, Q16, Q17, Q18)
  • Open — scope and prerequisites (Q9, Q10, Q12, Q13, Q15, Q20)
  • Resolved decisions (Q1, Q2, Q5, Q6, Q8, Q11, Q14, Q19)

Open tables now show only items that still need input. Resolved items are in a separate table with their decision rationale preserved for reference. Dropped the Status column from the open tables since everything there is open by definition.

Rajdeepc and others added 2 commits April 24, 2026 16:31
- Resolve Q1 (FocusgroupNavigationController merged), Q2 (Focusable
  mixin not needed), Q6 (deferred with overflow), Q8 (moot per Q18),
  Q11 (internal DOM not a consumer concern)
- Update Q4 with Figma/React research, Q7 with JSDoc standards
  rationale, Q12 with forced-colors guidance, Q18 downgraded from
  blocking
- Restructure open questions into three tables: open architecture,
  open scope, and resolved decisions (per rubencarvalho feedback)
- Update TL;DR, methods table, and blocking questions list accordingly

Made-with: Cursor
@caseyisonit caseyisonit dismissed their stale review April 27, 2026 16:02

dismissing my review since three others are approved. dont want to block.

@Rajdeepc Rajdeepc merged commit 72a34ee into main Apr 28, 2026
28 checks passed
@Rajdeepc Rajdeepc deleted the rajdeepc/docs-tabs-migration-plan branch April 28, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2nd gen These issues or PRs map to our 2nd generation work to modernizing infrastructure. Component:Tabs Status:Ready for review PR ready for review or re-review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants