-
Notifications
You must be signed in to change notification settings - Fork 140
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
Added autofocus feature to input components in input.py - (09/15/24). #788
base: master
Are you sure you want to change the base?
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.
Hi @StefanoCandiani! Thanks a lot for the PR.
I had two comments:
-
I think the user-facing name of the attribute should be
autofocus
, since it more clearly communicates the intended use of the attribute and matches the HTML and Vuetify attribute. -
We should elaborate on the use in the docstring. I'd like to see:
- That the element is focused on page load or when it is first mounted (in a
solara.lab.ConfirmationDialog
,solara.lab.Menu
, or inside an if statement, for example), and - That only one element per page (or more properly per element that gets mounted, I guess) should have
autofocus=True
mentioned.
- That the element is focused on page load or when it is first mounted (in a
Would this be fine? I changed |
How can I fix this issue? It seems that a specific version is unable to work but I'm unsure how to make it work? Does it just not support autofocus features in that version? Thanks. |
The failure in CI was a fluke. This is super cool. Did someone check this with a dialog? Does this work as expected? And should we have tests? |
@maartenbreddels I did manually verify it works as expected for menus, dialogs, initial load, and conditional rendering. If there are any things, let me know. For reference, as per MDN docs autofocus should focus the element on initial page load, or when the |
It seems vuetify is not using a dialog element, but just a div: https://vuetifyjs.com/en/components/dialogs/#usage (vuetify 2 and3). So, the user is responsible for making sure autofocus for a text element is not set, if a vuetify dialog with autofocus is used/open. |
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.
We discussed this with @maartenbreddels and thought it would be a good idea to also include a simple example of autofocus
being used in the documentation page for input components (you can add it to solara/website/pages/documentation/components/input/input.py, see button.py for an example).
Something to the tune of
import solara
import solara.lab
@solara.component
def Page():
show_dialog = solara.use_reactive(False)
solara.InputText("Type here", autofocus=True)
solara.Button("Show dialog", on_click=lambda: show_dialog.set(True))
with solara.lab.ConfirmationDialog(open=show_dialog):
solara.InputFloat("Float here", autofocus=True)
Feel free to get creative with the example and make it yours!
A bit off topic, but I just want to share this: |
As detailed in Issue #775 (Support Autofocus), I believe I added functionality of autofocus to each of the input functions (InputText, InputFloat, InputInt, and InputNumeric). Each of these will have a default parameter called
focus
that is initially set to False though the user can set to true or false. For InputText and InputNumeric, the focus variable is directly linked to the TextField autofocus default variable while for InputFloat and InputInt, thefocus
variable is merely passed on when the InputNumeric is called in the return statement. This is my first ever contribution so I'm not sure if I did it correctly or not so please let me know if I did a mistake or if I can optimize the code. Thank you!