Skip to content
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

new SelectOptions control for select options #443

Merged
merged 5 commits into from
May 29, 2022

Conversation

bharatkashyap
Copy link
Member

Fixes #393

@render
Copy link

render bot commented May 19, 2022

@oliviertassinari oliviertassinari temporarily deployed to select-prop-control - toolpad-db PR #443 May 19, 2022 12:34 — with Render Destroyed
@bharatkashyap
Copy link
Member Author

Screen.Recording.2022-05-19.at.6.12.01.PM.mov

Other possible UI options: Creating Chips when a user presses the space bar to create an option, but I felt like the text editor is less friction in cases when someone would prefer pasting in a lot of options

@apedroferreira
Copy link
Member

apedroferreira commented May 19, 2022

Hey Bharat, a UI suggestion that came to mind when looking at the video:

  • Maybe we should clear the textbox when you submit?
    Maybe we could make it possible to submit the options with a "confirm" button in the modal or something like that?

I feel like it would be a quicker option to understand than pressing the green checkmark (which is still a good option to keep in my opinion). (Sorry, i changed my mind about my previous suggestion since we're not closing the modal when we submit.)

@apedroferreira
Copy link
Member

apedroferreira commented May 19, 2022

Hey Bharat, a UI suggestion that came to mind when looking at the video:

  • Maybe we should clear the textbox when you submit?
    Maybe we could make it possible to submit the options with a "confirm" button in the modal or something like that?

I feel like it would be a quicker option to understand than pressing the green checkmark (which is still a good option to keep in my opinion). (Sorry, i changed my mind about my previous suggestion since we're not closing the modal when we submit.)

Or perhaps we could actually have a "confirm" button to confirm adding/deleting the options (which would close the modal too), and until you pressed it all the changes you made would be temporary.

@Janpot
Copy link
Member

Janpot commented May 19, 2022

  • I'm not a big fan of parsing the commas. What if I want an option that contains a comma. Perhaps pressing "enter" creates an option but keeps the input focused so you can type the next option?
  • How would one set a label for an option?

@bharatkashyap
Copy link
Member Author

  • I'm not a big fan of parsing the commas. What if I want an option that contains a comma. Perhaps pressing "enter" creates an option but keeps the input focused so you can type the next option?
  • How would one set a label for an option?
  • For the first, it's a trade off between able to accept a large list of options easily (a character separated list pasted in) versus being able to accept the separator character (comma in this case) in the options.

  • If we want to let users provide labels and values to options manually, then we probably need an alternate flow entirely - similar to the column addition flow with a menu in it. Is it likely that this would be what the user is intending to do when providing options manually?

@Janpot
Copy link
Member

Janpot commented May 20, 2022

For the first, it's a trade off between able to accept a large list of options easily (a character separated list pasted in) versus being able to accept the separator character (comma in this case) in the options.

I'm fine with using newlines as delimiter when pasting text in the box

If we want to let users provide labels and values to options manually, then we probably need an alternate flow entirely - similar to the column addition flow with a menu in it. Is it likely that this would be what the user is intending to do when providing options manually?

In any case, adding a label should be optional.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 22, 2022
@render
Copy link

render bot commented May 25, 2022

@oliviertassinari oliviertassinari temporarily deployed to select-prop-control - toolpad-db PR #443 May 25, 2022 10:10 — with Render Destroyed
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 25, 2022
@bharatkashyap
Copy link
Member Author

Screen.Recording.2022-05-25.at.3.41.33.PM.mov

(Note that I've fixed the issue of the last edited option opening up again on reopening the dialog which appears in the video)

@apedroferreira
Copy link
Member

(Note that I've fixed the issue of the last edited option opening up again on reopening the dialog which appears in the video)

Overall looks great, Bharat!
One thing that maybe as a user I wouldn't be able to find out is how to edit the options - what if there was an "edit" button/icon on the left of the delete button/icon?

@bharatkashyap
Copy link
Member Author

(Note that I've fixed the issue of the last edited option opening up again on reopening the dialog which appears in the video)

Overall looks great, Bharat! One thing that maybe as a user I wouldn't be able to find out is how to edit the options - what if there was an "edit" button/icon on the left of the delete button/icon?

I agree and I tried it, but it did feel a bit redundant to have two ways to open the edit flow for each option. So:

  1. Would we also open the Edit flow on the option click still, or it would open only on the icon click?

  2. If so, should we replicate this behaviour in the Columns editor as well, for consistency's sake? Might be too early to think of consistency in our UX patterns but perhaps a good practice to have.

@apedroferreira
Copy link
Member

(Note that I've fixed the issue of the last edited option opening up again on reopening the dialog which appears in the video)

Overall looks great, Bharat! One thing that maybe as a user I wouldn't be able to find out is how to edit the options - what if there was an "edit" button/icon on the left of the delete button/icon?

I agree and I tried it, but it did feel a bit redundant to have two ways to open the edit flow for each option. So:

  1. Would we also open the Edit flow on the option click still, or it would open only on the icon click?
  2. If so, should we replicate this behaviour in the Columns editor as well, for consistency's sake? Might be too early to think of consistency in our UX patterns but perhaps a good practice to have.

I see what you mean about the UX consistency, maybe clicking on the element you want to edit is a more consistent pattern with how the editor works... On second thought I think this behavior may be fine after all in my opinion.

@bharatkashyap bharatkashyap requested a review from Janpot May 26, 2022 10:01
Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

great!

@bharatkashyap bharatkashyap merged commit 7e4e303 into master May 29, 2022
@bharatkashyap bharatkashyap deleted the select-prop-control branch May 29, 2022 08:36
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.

Create property editor for Select options
4 participants