Skip to content

Convert comboboxes to inputs for SLDS2 #494

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

Merged
merged 7 commits into from
Jul 29, 2025

Conversation

msmx-mnakagawa
Copy link
Collaborator

@msmx-mnakagawa msmx-mnakagawa commented Jul 24, 2025

What I did

  • convert tags of elements with role="combobox" to <input type="text" readonly />s
  • extract text contents in findSelectedItemLabel() in Picklist

@msmx-mnakagawa msmx-mnakagawa self-assigned this Jul 24, 2025
Copy link

reg-suit bot commented Jul 24, 2025

✨✨ That's perfect, there is no visual difference! ✨✨

Check out the report here.

@msmx-mnakagawa msmx-mnakagawa requested a review from stomita July 24, 2025 22:44
@msmx-mnakagawa msmx-mnakagawa marked this pull request as ready for review July 24, 2025 22:44
@msmx-mnakagawa msmx-mnakagawa marked this pull request as draft July 24, 2025 23:03
@msmx-mnakagawa msmx-mnakagawa force-pushed the support-slds-2-convert-comboboxes-to-buttons branch from bab5edb to 6314b0d Compare July 24, 2025 23:04
@msmx-mnakagawa msmx-mnakagawa marked this pull request as ready for review July 24, 2025 23:08
@msmx-mnakagawa msmx-mnakagawa changed the title Convert comboboxes to buttons for SLDS2 Convert comboboxes to buttons for SLDS2 Jul 25, 2025
@msmx-mnakagawa msmx-mnakagawa changed the title Convert comboboxes to buttons for SLDS2 Convert comboboxes to buttons for SLDS2 Jul 25, 2025
@@ -161,7 +162,7 @@ const LookupSelectedState: FC<LookupSelectedStateProps> = ({
aria-expanded='false'
>
<span className='slds-truncate'>{selected.label}</span>
</div>
</button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are u willing to change back to button, not the div ? Have u checked the focus difference ?

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, it would be better to use <input type="text" readonly /> instead of changing back to button.
I did it in 4adb664.

Furthermore, I added a stringify logic in 6ab7b63.
I would appreciate if you confirm it as well.

@msmx-mnakagawa msmx-mnakagawa changed the title Convert comboboxes to buttons for SLDS2 Convert comboboxes to inputs for SLDS2 Jul 25, 2025
@msmx-mnakagawa msmx-mnakagawa force-pushed the support-slds-2-convert-comboboxes-to-buttons branch from c04117d to 4adb664 Compare July 29, 2025 02:57
@msmx-mnakagawa msmx-mnakagawa marked this pull request as draft July 29, 2025 03:10
@msmx-mnakagawa msmx-mnakagawa requested a review from stomita July 29, 2025 03:10
@msmx-mnakagawa msmx-mnakagawa marked this pull request as ready for review July 29, 2025 03:11
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.

Issues when opening picklist dropdown

Behavior before merging support-slds-2 PR #485
CleanShot 2025-07-29 at 12 23 58@2x

After:
CleanShot 2025-07-29 at 12 27 37@2x

It also shows pointer cursor when hovered to the control in previous, but not shown now.

@msmx-mnakagawa
Copy link
Collaborator Author

msmx-mnakagawa commented Jul 29, 2025

@stomita

Issues when opening picklist dropdown

If you mean the widths of the picklist items, #495 is resolving it.
If you are concerned about this viewpoint, it would be better to confirm #495 at first.

It also shows pointer cursor when hovered to the control in previous, but not shown now.

I fixed it in b226640.

@msmx-mnakagawa msmx-mnakagawa requested a review from stomita July 29, 2025 04:02
function useInitComponentStyle() {
useEffect(() => {
registerStyle('picklist', [
['.react-picklist-input:focus-visible', '{ outline: none; }'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you are willing to cancel the focus ring of the control, it is not working right now.
I think it is not required to remove the ring for the focused picklist.

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
I got it.
I fixed it in 42543c5.

@msmx-mnakagawa msmx-mnakagawa requested a review from stomita July 29, 2025 04:23
@stomita stomita merged commit 19a6b74 into master Jul 29, 2025
2 checks passed
@msmx-mnakagawa msmx-mnakagawa deleted the support-slds-2-convert-comboboxes-to-buttons branch July 29, 2025 05:54
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