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

Enhancement: Add focus event to textbox and number component #5005

Merged
merged 16 commits into from Aug 1, 2023

Conversation

JodyZ0203
Copy link
Contributor

@JodyZ0203 JodyZ0203 commented Jul 24, 2023

Description

  • Registered event called Focusable under event.py and combined Blurrable with Focusable
  • Updated the corresponding components (textbox & numbers) to reflect the changes in the frontend through .focus() event.
  • Added focus event to ColorPicker and DropDown Components

Closes: #(issue)
This PR fixes #4817

🎯 PRs Should Target Issues

Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.

Not adhering to this guideline will result in the PR being closed.

Tests

  1. PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests: bash scripts/run_all_tests.sh

  2. You may need to run the linters: bash scripts/format_backend.sh and bash scripts/format_frontend.sh

Test with

import gradio as gr


def greet(str):
    return str

with gr.Blocks() as demo:
    with gr.Row():
            text1 = gr.TextArea()
            text2 = gr.TextArea()
            drop1 = gr.Dropdown(choices=['1','2','3','4','5'], label="Dropdown with Blur")
            drop2 = gr.Dropdown(
                choices=[
                    "simple",
                    "stacked",
                    "grouped",
                    "simple-horizontal",
                    "stacked-horizontal",
                    "grouped-horizontal",
                ],
                type="value",
                label="Dropdown with Focus",
            )
            colorPicker = gr.ColorPicker(label="ColorPicker with Blur")
            colorPicker2 = gr.ColorPicker(label="ColorPicker with Focus")
    drop1.blur(greet, text1, text2)
    drop2.focus(greet, text1, text2)
    colorPicker.blur(greet, text1, text2)
    colorPicker2.focus(greet, text1, text2)


if __name__ == "__main__":
    demo.launch()

Demo

Gradio.Component.Focus.mov
Focusable.on.Dropdown.and.ColorPicker.mov

@vercel
Copy link

vercel bot commented Jul 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
gradio ✅ Ready (Inspect) Visit Preview Aug 1, 2023 6:08pm

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Jul 24, 2023

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
@gradio/app minor
@gradio/form minor
gradio minor
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Enhancement: Add focus event to textbox and number component

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Jul 24, 2023

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


You can install the changes in this PR by running:

pip install https://gradio-builds.s3.amazonaws.com/3af551af4b7979636a8fdd449c818450bfc39ce2/gradio-3.39.0-py3-none-any.whl

@aliabid94
Copy link
Collaborator

Makes sense (surprised we didn't have this already given we have Blurable). Thanks for the PR @JodyZ0203!

What if we merge Focasable and Blurable into a single class Focusable that has both .focus and .blur? (Analogous to how any Playable has .play, .pause, and .stop) We'd have to add .focus to dropdown and colorpicker too.

@JodyZ0203
Copy link
Contributor Author

JodyZ0203 commented Jul 25, 2023

Makes sense (surprised we didn't have this already given we have Blurable). Thanks for the PR @JodyZ0203!

What if we merge Focasable and Blurable into a single class Focusable that has both .focus and .blur? (Analogous to how any Playable has .play, .pause, and .stop) We'd have to add .focus to dropdown and colorpicker too.

That's a good idea. Since they are similar enough, I can merge them together.

@abidlabs
Copy link
Member

Hi @JodyZ0203 thanks for the contribution! Would you be interested in implementing the suggested changes?

@JodyZ0203
Copy link
Contributor Author

Merge branch 'main' into feat/detect_textbox_focus

Hi @abidlabs , I'm currently working on the suggested changes. Aiming to get PR out this weekend. Thanks!

@JodyZ0203
Copy link
Contributor Author

@abidlabs @aliabid94 Changes are ready!

@abidlabs
Copy link
Member

Thanks @JodyZ0203! Would you just be able to resolve the merge conflict, and then happy to take another look

@JodyZ0203
Copy link
Contributor Author

Hi @abidlabs! Merge conflict resolved. It's ready for another look. Thanks!

@@ -22,6 +23,15 @@
dispatch("input");
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

no need for event listener methods if they just dispatch - on:blur and on:focus is sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That make sense.

function handle_focus(e: FocusEvent){
dispatch("focus");
showOptions = !showOptions;
if (showOptions) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

there's an existing handleFocus method, that is pretty identical except you changed
filtered = choices;
to
inputValue = "";

any reason for this?

Copy link
Member

@abidlabs abidlabs Aug 1, 2023

Choose a reason for hiding this comment

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

Yeah we should keep filtered = choices; I believe, otherwise, it'll revert some of the fixes in #5039

@aliabid94
Copy link
Collaborator

made the fixes, should be ready to merge. Thanks for this @JodyZ0203 !

@abidlabs
Copy link
Member

abidlabs commented Aug 1, 2023

Thanks again @JodyZ0203! Merging

@abidlabs abidlabs merged commit f5539c7 into gradio-app:main Aug 1, 2023
12 checks passed
@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Aug 1, 2023

🎉 Chromatic build completed!

There are 4 visual changes to review.
There are 0 failed tests to fix.

@pngwn pngwn mentioned this pull request Aug 1, 2023
@JodyZ0203
Copy link
Contributor Author

Thank you! @abidlabs @aliabid94

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.

Detect textbox focus
4 participants