-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Introduce custom select component #10991
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looking pretty awesome. This is something I've been looking forward to / meaning to do for a while, so I'm really excited to see it!
(PS: everything I say about VSCode's behavior applies in the preference case - I haven't checked the other use cases as thoroughly)
packages/preferences/src/browser/views/components/preference-select-input.ts
Outdated
Show resolved
Hide resolved
packages/preferences/src/browser/views/components/preference-select-input.ts
Outdated
Show resolved
Hide resolved
packages/preferences/src/browser/views/components/preference-select-input.ts
Outdated
Show resolved
Hide resolved
0c6d716
to
8b68001
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look really great and are a very nice improvement to the framework 👍
I do not have any feedback for the code itself at the time, I mostly tested the functionality and each dropdown, especially how the changes enhance the UX of the preferences-view and now display enumDescriptions
.
The only minor UI improvement I could see at the time is to reduce the separator height, which can be seen with debug configurations dropdown:
It is debatable however if you believe the thicker border looks better than the behavior we see in vscode.
@vince-fugnitto I think you're right. I reduced the separator thickness to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good, and work well for me 👍
@colin-grant-work I would merge this within the next few days. Any other comments from your side? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Very nice work @msujew!
What it does
Closes #8153
Introduces a custom react component for rendering
select
elements. It mirrors mostly how a native htmlselect
element works and allows for some additional features like element descriptions, details, separators, etc.Mostly get's used for preference enum rendering, but I've also replaced some other
select
elements throughout the app.How to test
In general, test that the component works just like the VSCode select element for enum preferences, launch debug config selection, etc.
Review checklist
Reminder for reviewers