Update `Lookup` for SLDS2 #482

msmx-mnakagawa · 2025-06-05T06:45:40Z

What I did

Lookup
- update markups significantly
- improve a11y
Spinner
- add a layout prop
- add an x-small size
Icon
- add an xx-small size

Reference

https://v1.lightningdesignsystem.com/components/lookups/

reg-suit · 2025-06-05T06:46:42Z

reg-suit detected visual differences.

Check this report, and review them.

🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴

🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵

What do the circles mean?

The number of circles represent the number of changed images.
🔴 : Changed items, ⚪ : New items, ⚫ : Deleted items, and 🔵 Passed items

How can I change the check status?

If reviewers approve this PR, the reg context status will be green automatically.

- add SLDS-compliant combobox markup structure - implement basic aria attributes (role, aria-expanded, aria-haspopup, etc.) - add keyboard navigation (arrow keys, Enter, Escape) - support search functionality with dropdown display - include listHeader and listFooter support - add loading state display - maintain existing API compatibility for basic features - temporarily comment out future features in stories to avoid type errors

- add unified a11y ID generation (labelId, scopeId) - implement proper label-input association with FormElement - add common a11y props (ariaLabelledBy, ariaControls, ariaHaspopup) - improve ID generation with fallbackId pattern - enhance WAI-ARIA compliance for better screen reader support

- add scope-related props (scopes, targetScope, onScopeSelect, etc.) - implement scope state management with controlled values - add scope selection UI with icon-only display (slds-has-icon-only) - implement scope dropdown with entity icons and selection states - add blur-to-close functionality for scope dropdown - support Multi Entity Lookup pattern with combobox-group layout - maintain existing API compatibility for single-scope lookups

- replace manual slds-icon_container spans with container={true} - unify data item icons to use Icon component container prop - unify search icon to use Icon component container prop - unify scope dropdown icon to use Icon component container prop - simplify code by leveraging Icon component functionality - ensure consistent Icon usage across all components

- add selected state combobox with SLDS-compliant markup structure - implement entity icon and selected value display with slds-media layout - add remove selection button with close icon and accessibility - use slds-lookup__result-text for selected value display - implement conditional rendering between selected and search states - add onRemoveSelection handler for clearing selection - maintain proper aria attributes for selected state combobox - follow reference markup pattern for selected state display

- add iconAlign prop to control search icon position (left/right) - implement dynamic className generation for icon positioning - add slds-input-has-icon_left/right and slds-input__icon_left/right classes - restore Stories functionality by uncommenting scope-related features - restore iconAlign: 'left' in WithSearchIconInLeft story - restore scopes: LOOKUP_SCOPES in MultiScope story - restore scope event handlers in LookupControlled component - complete all planned Lookup component features and improvements

stomita

Too small icon and maybe bigger down icon of scope dropdown button, comparing to the standard multi-scope lookup

Standard lookup field of multi scope:

stomita

Scope dropdown of lookup should be keyboard navigatable (by up/down arrow, return key, esc key, and so on), like current dropdown button implementation.

msmx-mnakagawa · 2025-06-19T01:26:14Z

@stomita

Too small icon and maybe bigger down icon of scope dropdown button, comparing to the standard multi-scope lookup

I've tried to tweak the layout as below.
Could you confirm again...?

msmx-mnakagawa · 2025-06-19T01:30:23Z

@stomita

Scope dropdown of lookup should be keyboard navigatable (by up/down arrow, return key, esc key, and so on), like current dropdown button implementation.

I've tried to add keyboard handlers, so could you take a look again...?

stomita · 2025-06-19T02:33:30Z

src/scripts/Lookup.tsx

+                </ul>
+              </div>
+            )}
+          </div>


Divide into sub components as the former implementation does, not having large JSX tree.

@stomita
I got it.
I've divided the parent one to sub components in 2b8cfec.

msmx-mnakagawa · 2025-06-19T06:22:51Z

FYI, I've removed aria-labelledby in 5edc2d8, reflecting #472 (comment).

stomita · 2025-06-19T07:22:03Z

src/scripts/Lookup.tsx

-  const onSelect = useEventCallback((entry: LookupEntry | null) => {
-    onSelect_?.(entry);
-  });
+  const renderListHeader = () => {


No need to be a function, just assign to var like const listHeaderItem = !listHeader ? null : <li ... /> or simply inlining with root tree would be enough.

@stomita
I fixed it in bf62197.

stomita · 2025-06-19T07:31:23Z

src/scripts/Lookup.tsx

+            setScopeOpened(!scopeOpened);
+          }
+        },
+        isIgnoreTabNavigation: () => {


In terms of naming conventions, isTabNavigationIgnored or ignoreTabNavigation would be more grammatically appropriate.

@stomita
I changed the naming with isTabNavigationIgnored in 2078825.

stomita · 2025-06-19T07:38:12Z

src/scripts/Lookup.tsx

+              setScopeFocusedIndex(-1);
+            } else {
+              const prevIndex = Math.max(scopeFocusedIndex - 1, 0);
+              setScopeFocusedIndex(prevIndex);


This approach is setting react state and triggers re-render to Lookup component, which may or may not introduce some difficulties (especially regarding to the focus of elements). If you are going to keep the focus index in state, scoping the handler to a hook and capsulate it to the sub component that involves the focus would be preferable.

@stomita
I tried to restrict the scope of state changes in 3664759.
Does my understanding match with yours?

stomita · 2025-06-19T07:41:11Z

src/scripts/Lookup.tsx

-      e.preventDefault();
-      e.stopPropagation();
-      onResetSelection?.();
+const createKeyHandler = (config: KeyHandlerConfig) => {


This logic can be directly implemented using hook.

@stomita
I reimplemented it as a hook in 4ef6024.

msmx-mnakagawa self-assigned this Jun 5, 2025

msmx-mnakagawa requested a review from stomita June 5, 2025 06:49

msmx-mnakagawa mentioned this pull request Jun 5, 2025

Support SLDS2 #485

Draft

42 tasks

msmx-mnakagawa added 10 commits June 13, 2025 13:50

(Icon): add xx-small size

0746be9

(Spinner): add x-small size

7dbc121

(Spinner): add a layout prop

0d68f52

(Lookup): apply rest updates

296b514

msmx-mnakagawa force-pushed the support-slds-2-lookup branch from 289545a to 296b514 Compare June 13, 2025 05:27

msmx-mnakagawa changed the base branch from support-slds-2-form-element-field-set to support-slds-2 June 13, 2025 05:27

msmx-mnakagawa marked this pull request as ready for review June 13, 2025 05:47

stomita requested changes Jun 17, 2025

View reviewed changes

msmx-mnakagawa added 11 commits June 18, 2025 18:06

Merge branch 'support-slds-2' into support-slds-2-lookup

722c222

(Lookup): make getPrevValue() and getNextValue() not wrap around

1750dd0

(Lookup): add Tab / Shift + Tab support

2352af6

(Lookup): add a scrollIntoView() behavoir

87d3bef

(Lookup): add onScopeKeyDown()

76aaf73

(Lookup): refactor scrollIntoView() logic

3c7145a

(Lookup): refactor keyboard handlers

4dc553d

(Lookup): tweak icon sizes

20eff7f

(Lookup): refactor to use optionIdPrefix

a3df654

(Lookup): add a space key support

1cf0c6e

(Lookup): enable skipping selections

9f48228

Merge branch 'support-slds-2' into support-slds-2-lookup

a61c532

msmx-mnakagawa requested a review from stomita June 19, 2025 01:30

stomita requested changes Jun 19, 2025

View reviewed changes

msmx-mnakagawa added 2 commits June 19, 2025 14:01

(Lookup): refactor to divide sub components

2b8cfec

Merge branch 'support-slds-2' into support-slds-2-lookup

6eadf5e

msmx-mnakagawa requested a review from stomita June 19, 2025 05:51

msmx-mnakagawa added 3 commits June 19, 2025 15:04

(Lookup): remove aria-labelledby for consistency

5edc2d8

(Lookup): remove scopeId that wouldn't be necessary

78a7f0f

(Lookup): pass comboboxId as controlId

417ca6e

msmx-mnakagawa marked this pull request as draft June 19, 2025 06:09

msmx-mnakagawa marked this pull request as ready for review June 19, 2025 06:22

stomita requested changes Jun 19, 2025

View reviewed changes

msmx-mnakagawa added 6 commits June 19, 2025 17:47

Merge branch 'support-slds-2' into support-slds-2-lookup

0cdebf9

(Lookup): tweak keyboeard logic

4c586c0

(Lookup): put divided components directly

bf62197

(Lookup): rename isIgnoreTabNavigation with isTabNavigationIgnored

2078825

(Lookup): reimplement createKeyHandler() with a hook

4ef6024

(Lookup): (scope) mitigate re-rendering by capsulating state changes

3664759

msmx-mnakagawa force-pushed the support-slds-2-lookup branch from 626fa5c to 3664759 Compare June 20, 2025 02:08

msmx-mnakagawa requested a review from stomita June 20, 2025 02:14

Update Lookup for SLDS2 #482

Are you sure you want to change the base?

Update Lookup for SLDS2 #482

Conversation

msmx-mnakagawa commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Reference

Uh oh!

reg-suit bot commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stomita left a comment

Choose a reason for hiding this comment

Uh oh!

stomita left a comment

Choose a reason for hiding this comment

Uh oh!

msmx-mnakagawa commented Jun 19, 2025

Uh oh!

msmx-mnakagawa commented Jun 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msmx-mnakagawa commented Jun 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Update `Lookup` for SLDS2 #482

Update `Lookup` for SLDS2 #482

msmx-mnakagawa commented Jun 5, 2025 •

edited

Loading

reg-suit bot commented Jun 5, 2025 •

edited

Loading