Skip to content

feat(color-loupe): migrate sp-color-loupe component to 2nd-gen#6147

Open
blunteshwar wants to merge 9 commits intocolor-loupe-migrationfrom
claude-do-the-honors
Open

feat(color-loupe): migrate sp-color-loupe component to 2nd-gen#6147
blunteshwar wants to merge 9 commits intocolor-loupe-migrationfrom
claude-do-the-honors

Conversation

@blunteshwar
Copy link
Copy Markdown
Contributor

@blunteshwar blunteshwar commented Apr 7, 2026

Description

Migrates the color-loupe component from 1st-gen (sp-color-loupe) to 2nd-gen (swc-color-loupe) following the washing machine workflow. All 7 migration steps are complete and the status table has been updated.

What changed

Core package (2nd-gen/packages/core/components/color-loupe/)

  • ColorLoupe.types.ts — minimal types file with the default color constant
  • ColorLoupe.base.ts — abstract base class extending SpectrumElement; holds the two public properties (open, color)
  • index.ts — re-exports

SWC package (2nd-gen/packages/swc/components/color-loupe/)

  • ColorLoupe.ts — concrete class with render() and styles
  • color-loupe.css — token-based CSS following the S2 migration guide
  • index.tsdefineElement('swc-color-loupe', ColorLoupe) registration
  • stories/color-loupe.stories.ts — Playground, Overview, Anatomy, States, ColorDisplay (behaviors), and Accessibility stories
  • test/color-loupe.test.ts — Storybook play-function tests (defaults, open reflection, color property)
  • test/color-loupe.a11y.spec.ts — Playwright attribute test asserting aria-hidden="true" on the SVG (replaced the unusable empty toMatchAriaSnapshot which Playwright rejects even for legitimately empty trees)

Migration docs (CONTRIBUTOR-DOCS/03_project-planning/03_components/color-loupe/)

  • rendering-and-styling-migration-analysis.md — full API surface, DOM structure, factor assessment, and gen-2 delta notes (moved from 1st-gen/packages/color-loupe/MIGRATION_ANALYSIS.md)
  • accessibility-migration-analysis.md — updated
  • migration-checklist.md — all 7 steps marked complete

Status table

  • CONTRIBUTOR-DOCS/.../01_status.md — all 7 columns marked for Color Loupe

Notable changes from 1st-gen

Area Change
SVG path Fixed double-close-command bug (61.575ZZ61.575Z)
SVG mask Fixed broken mask reference (xlink:href="#path"href="#loupe-path")
xlink:href Migrated deprecated xlink:href to modern href throughout the SVG
Opacity checkerboard Replaced @spectrum-web-components/opacity-checkerboard import with an inline repeating-conic-gradient using --swc-opacity-checkerboard-* tokens and light-dark() for theme support
Drop shadow Migrated from S1 --spectrum-drop-shadow-x to S2 token("drop-shadow-elevated-*")
Token syntax All --spectrum-color-loupe-* / --mod-colorloupe-* chains replaced with token() and --swc-color-loupe-* exposed overrides
Color display Simplified render: separate div.swc-ColorLoupe-colorFill with background: ${this.color} instead of setting --spectrum-picked-color on the SVG inline style
Factor step Intentionally skipped — component is pure-presentational (~40 lines of logic), no separable state

Motivation and context

sp-color-loupe is a dependency of the color picker family. Migrating it to 2nd-gen unblocks downstream component migrations (swc-color-field, swc-color-slider, swc-color-area) and ensures it consumes Spectrum 2 design tokens.

Related issue(s)

  • Jira: SWC-1193 (WCAG 1.4.11 limitation — non-text contrast for the loupe border, documented in the accessibility analysis, not resolved in this PR)

Screenshots (if appropriate)

Visual verification via the color-loupe--overview Storybook story. No screenshot attached; teardrop shape, checkerboard, and color fill are visually unchanged from 1st-gen.

Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes.
  • I have included a well-written changeset if my change needs to be 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

  • Loupe renders correctly in Storybook

    1. Go to Color Loupe — Overview
    2. Verify the teardrop shape, checkerboard pattern, and blue color fill all render
    3. Expect pixel-accurate match against the 1st-gen sp-color-loupe
  • Open/close animation works

    1. Go to Color Loupe — States
    2. Toggle the open control in the Storybook controls panel
    3. Expect the loupe to animate in (opacity + transform) when open and animate out when closed
  • Color fill updates correctly

    1. Go to Color Loupe — Color display
    2. Change the color control to a color with alpha (e.g., rgba(255, 0, 0, 0.5))
    3. Expect the checkerboard to show through the transparent fill
  • Lint and unit tests pass

    1. Run yarn lint:2nd-gen
    2. Run yarn test:unit:2nd
    3. Expect zero errors
  • Playwright a11y tests pass

    1. Run yarn test:a11y:2nd
    2. Expect both color-loupe.a11y.spec.ts tests to pass — they assert aria-hidden="true" on the SVG element

Device review

  • Did it pass in Desktop?
  • Did it pass in (emulated) Mobile?
  • Did it pass in (emulated) iPad?

Accessibility testing checklist

Required: Complete each applicable item and document your testing steps.

  • Keyboard (non-interactive component — no focusable parts)

    1. Go to Color Loupe — Accessibility
    2. Press Tab repeatedly through the story
    3. Expect focus to skip the loupe entirely — it has no tab stop and sets no tabindex
    4. Confirm no regressions in surrounding Storybook chrome
  • Screen reader (visual-only component — SVG is aria-hidden)

    1. Open Color Loupe — Accessibility with VoiceOver (macOS) or NVDA (Windows)
    2. Navigate through the page with VO+Right / F6
    3. Expect the loupe SVG to be completely skipped — no role, name, or value should be announced
    4. Confirm the swc-color-loupe custom element itself is not announced as an interactive control

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 7, 2026

⚠️ No Changeset found

Latest commit: 50b7660

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

@blunteshwar blunteshwar added the Status:WIP PR is a work in progress or draft label Apr 7, 2026
@Rajdeepc
Copy link
Copy Markdown
Contributor

@blunteshwar Anything blocking this migration?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 16, 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-6147

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.

@blunteshwar blunteshwar marked this pull request as ready for review April 17, 2026 04:55
@blunteshwar blunteshwar requested a review from a team as a code owner April 17, 2026 04:55
@blunteshwar blunteshwar changed the title feat(color-loupe): implement Color Loupe component feat(color-loupe): migrate sp-color-loupe to swc-color-loupe (2nd-gen) Apr 17, 2026
@blunteshwar blunteshwar 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 17, 2026
@Rajdeepc Rajdeepc changed the title feat(color-loupe): migrate sp-color-loupe to swc-color-loupe (2nd-gen) feat(color-loupe): migrate sp-color-loupe component to 2nd-gen Apr 17, 2026
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.

Requesting changes because it looks like the implementation could be upgraded to match S2 Spectrum CSS as noted.

class="swc-ColorLoupe-colorFill swc-ColorLoupe--clipped"
style="background: ${this.color}"
></div>
<svg aria-hidden="true" class="swc-ColorLoupe-svg" overflow="visible">
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.

There are some rendering issues with this SVG - I wonder if you could copy in from the Spectrum CSS spectrum-two version instead?

The loupe-path is not positioned correctly in this current version. Removing the transform at least in browser caused a different issue. I hid the color and checkerboard for this screenshot to better see the issue.

Image

* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

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.

As noted in the main file, consider updating the SVG implementation from the spectrum-two branch which also simplifies the CSS.

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

Labels

Status:Ready for review PR ready for review or re-review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants