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

Enable multi-select on gradio.Dropdown #2871

Merged
merged 45 commits into from Jan 5, 2023
Merged

Enable multi-select on gradio.Dropdown #2871

merged 45 commits into from Jan 5, 2023

Conversation

dawoodkhan82
Copy link
Collaborator

@dawoodkhan82 dawoodkhan82 commented Dec 21, 2022

Description

Add option to select multiple options on gr.Dropdown

Will be updating the colors to match other form components.

Screen.Recording.2023-01-03.at.3.12.44.PM.mov

Please include:

  • relevant motivation
  • a summary of the change
  • which issue is fixed.
  • any additional dependencies that are required for this change.

Closes: #1076

Checklist:

  • I have performed a self-review of my own code
  • I have added a short summary of my change to the CHANGELOG.md
  • My code follows the style guidelines of this project
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

A note about the CHANGELOG

Hello 👋 and thank you for contributing to Gradio!

All pull requests must update the change log located in CHANGELOG.md, unless the pull request is labeled with the "no-changelog-update" label.

Please add a brief summary of the change to the Upcoming Release > Full Changelog section of the CHANGELOG.md file and include
a link to the PR (formatted in markdown) and a link to your github profile (if you like). For example, "* Added a cool new feature by [@myusername](link-to-your-github-profile) in [PR 11111](https://github.com/gradio-app/gradio/pull/11111)".

If you would like to elaborate on your change further, feel free to include a longer explanation in the other sections.
If you would like an image/gif/video showcasing your feature, it may be best to edit the CHANGELOG file using the
GitHub web UI since that lets you upload files directly via drag-and-drop.

gradio/components.py Outdated Show resolved Hide resolved
@abidlabs
Copy link
Member

abidlabs commented Dec 21, 2022

Awesome, love the fact that the dropdown is typeable as well. Will be useful for very large number of choices.

Some additional suggestions on the UI / UX side:

  1. The appearance of Dropdown(multiselect=False) and Dropdown(multiselect=True) are very different. We should make them more consistent, with the only difference being that you can only choose a single element if multiselect=False.

image

image

For example, the container with the Dropdown label is now completely gone. We should bring that back. But also, the entire design of the list of choices is different, the textbox part is different, etc.

  1. You can't seem to currently use Dropdown(multiselect=False) as an output component the way you can with Dropdown(multiselect=True).

Try running:

import gradio as gr

with gr.Blocks() as demo:
    gr.Dropdown(["angola", "pakistan", "canada"], multiselect=True, value=["angola"])
    
demo.launch()

vs.

import gradio as gr

with gr.Blocks() as demo:
    gr.Dropdown(["angola", "pakistan", "canada"], multiselect=False, value="angola")
    
demo.launch()
  1. Typing doesn't filter choices. As I start typing "Pa", I would expect the choices to filter to just Pakistan, and that I could use the down arrow key and enter to go down and select it. However, this does not happen:

image

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

Copy link
Collaborator

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

@dawoodkhan82 This is so cool! It's great that developers can update whether or not multi-select is enabled via gr.Dropdown.update as well.

Left comments on some bugs I think I found.

In addition, I agree with @abidlabs comment about filtering with what's been typed and the look being too different from the existing dropdown.

def update(
value: Optional[Any] = _Keywords.NO_VALUE,
choices: Optional[str | List[str]] = None,
multiselect: bool = False,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better to make multiselect=None otherwise any update to a multi-select dropdown will switch off the multiselect

Copy link
Collaborator

Choose a reason for hiding this comment

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

We do not support update changing backend configuration yet, such as multiselect. update only allows changing frontend configuration. So please remove multiselect as an option to update

ui/packages/form/src/Dropdown.svelte Outdated Show resolved Hide resolved
ui/packages/form/src/Dropdown.svelte Show resolved Hide resolved
@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@github-actions
Copy link
Contributor

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

@aliabid94
Copy link
Collaborator

great work. styling is still a bit off, I'll just make changes directly to branch.

@aliabid94
Copy link
Collaborator

Just looking at this, I feel like should be a completely different element than Dropdown, e.g. a gr.Select. It has a different type (List of strings vs string), different purpose, and different visual layout than Dropdown. There's so little in common that all the logic branches whether or not it is a multiselect. @abidlabs @dawoodkhan82

@abidlabs
Copy link
Member

Just looking at this, I feel like should be a completely different element than Dropdown, e.g. a gr.Select. It has a different type (List of strings vs string), different purpose, and different visual layout than Dropdown. There's so little in common that all the logic branches whether or not it is a multiselect. @abidlabs @dawoodkhan82

From a user's perspective, it feels "natural" to me to have a multiselect variant of the dropdown alongside a regular single-select dropdown. But I don't have a strong opinion, I'm fine with a separate component. If we do end up making it a different component, I think gr.Multiselect is more of an appropriate name

@dawoodkhan82
Copy link
Collaborator Author

dawoodkhan82 commented Dec 30, 2022

Synced with @aliabid94 , there's changes he suggested which makes sense and I'll implement:

  • change the styling of the selected option bubbles to match the radio button
  • separate the front end code into a separate Multiselect.svelte component
  • re-implement the multiselect dropdown to use datalist
  • Maybe separate the component completely (frontend + backend)? Not sure about this, I think for the same reasons as @abidlabs mentioned, it makes sense from a developer's pov to have it be the same component.

@abidlabs
Copy link
Member

This is going to be a really slick component!

@dawoodkhan82
Copy link
Collaborator Author

synced with @aliabid94 and @pngwn. Decided to move forward with the custom drop-down. Will look into combining the custom dropdown with datalist to improve accessibility, along with the other forms.

@gradio-app gradio-app deleted a comment from github-actions bot Jan 3, 2023
@gradio-pr-bot
Copy link
Contributor

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-2871-all-demos

def update(
value: Optional[Any] = _Keywords.NO_VALUE,
choices: Optional[str | List[str]] = None,
multiselect: bool = False,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We do not support update changing backend configuration yet, such as multiselect. update only allows changing frontend configuration. So please remove multiselect as an option to update

ui/packages/form/src/MultiSelect.svelte Outdated Show resolved Hide resolved
ui/packages/form/src/MultiSelect.svelte Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@abidlabs abidlabs left a comment

Choose a reason for hiding this comment

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

Looks beautiful! Nit: can we add backend tests to ensure that preprocessing/postprocess multiselect works as we expect it to?

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Jan 4, 2023

🎉 The demo notebooks match the run.py files! 🎉

@dawoodkhan82 dawoodkhan82 merged commit 9fff1e0 into main Jan 5, 2023
@dawoodkhan82 dawoodkhan82 deleted the multiselect branch January 5, 2023 00:13
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.

Enable multi-select on gradio.Dropdown
7 participants