Skip to content

Update Picklist for SLDS2 #481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: support-slds-2
Choose a base branch
from

Conversation

msmx-mnakagawa
Copy link
Collaborator

@msmx-mnakagawa msmx-mnakagawa commented Jun 4, 2025

What I did

  • update markups to combobox pattern
  • update classnames for dropdown sizing
  • improve a11y for disabled state

Reference

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

@msmx-mnakagawa msmx-mnakagawa self-assigned this Jun 4, 2025
Copy link

reg-suit bot commented Jun 4, 2025

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.

@msmx-mnakagawa msmx-mnakagawa force-pushed the support-slds-2-picklist branch from 1847470 to 720fe6c Compare June 5, 2025 00:05
@msmx-mnakagawa msmx-mnakagawa requested a review from stomita June 5, 2025 06:49
@msmx-mnakagawa msmx-mnakagawa mentioned this pull request Jun 5, 2025
42 tasks
@msmx-mnakagawa msmx-mnakagawa force-pushed the support-slds-2-picklist branch from 720fe6c to 8270d7b Compare June 13, 2025 04:28
@msmx-mnakagawa msmx-mnakagawa changed the base branch from support-slds-2-form-element-field-set to support-slds-2 June 13, 2025 04:28
@msmx-mnakagawa msmx-mnakagawa force-pushed the support-slds-2-picklist branch from 8270d7b to 18274d8 Compare June 13, 2025 04:39
@msmx-mnakagawa msmx-mnakagawa marked this pull request as ready for review June 13, 2025 04:46
Copy link
Collaborator

@stomita stomita left a comment

Choose a reason for hiding this comment

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

@msmx-mnakagawa
Copy link
Collaborator Author

@stomita
Thank you for confirming.
I've tried to add a similar feature to what you mentioned in 5718342.

@msmx-mnakagawa msmx-mnakagawa requested a review from stomita June 18, 2025 03:36
@msmx-mnakagawa msmx-mnakagawa marked this pull request as draft June 19, 2025 01:19
@msmx-mnakagawa msmx-mnakagawa marked this pull request as ready for review June 19, 2025 01:24
@msmx-mnakagawa
Copy link
Collaborator Author

FYI:
The same behavior about VRT as DateInput would occur in this PR as well.
https://mashmatrix.github.io/react-lightning-design-system/?path=/story/picklist--default

focusedValue,
scrollFocusedElementIntoView,
]);

Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems you are controlling by calculating and setting scroll position, but it may be better to delegate the control with the browser's focus capability as the current DropdownMenu does.

Copy link
Collaborator Author

@msmx-mnakagawa msmx-mnakagawa Jun 19, 2025

Choose a reason for hiding this comment

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

@stomita
In ecf43a5, I've reintroduced DropdownMenu, just as it was before this PR.

It's likely that portalClassName makes VRT a little flaky though...

Copy link
Collaborator

Choose a reason for hiding this comment

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

@msmx-mnakagawa So you decided not changing the current slds-picklist to slds-combobox, right ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@stomita
As we discussed directly, I reverted the DropdownMenu solution.
After that, I added a simple .focus() logic, referring to DropdownMenu, in e1721e4.

You can already confirm this PR!

@msmx-mnakagawa msmx-mnakagawa force-pushed the support-slds-2-picklist branch from 94c80a9 to ecf43a5 Compare June 19, 2025 05:04
@msmx-mnakagawa msmx-mnakagawa requested a review from stomita June 19, 2025 05:59
@msmx-mnakagawa msmx-mnakagawa marked this pull request as draft June 19, 2025 06:09
@msmx-mnakagawa
Copy link
Collaborator Author

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

@msmx-mnakagawa msmx-mnakagawa marked this pull request as ready for review June 19, 2025 06:20
@msmx-mnakagawa
Copy link
Collaborator Author

FYI: I'm sorry I, added a small change de94337.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants