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

Option to disable step buttons on number_input #7154

Merged
merged 1 commit into from Aug 17, 2023

Conversation

denck007
Copy link
Contributor

Describe your changes

Add optional kwarg to st.number_input called disable_step_button which controls visibility of the +/- buttons next to the input field.

Motivation

We are using Streamlit to annotate images and have many numerical input fields the users must interact with. Allowing them to keep hands on keyboard with consistent tabbing behavior combined with forms greatly increases their throughput and minimizes errors.

For example, if you have this setup:

st.number_input("X", min_value=0, max_value=5)
st.number_input("Y", min_value=0, max_value=5)

entering a value of X=0 pressing TAB twice will get you to the Y input field. But entering X=1 requires pressing TAB 3x. By changing to :

st.number_input("X", min_value=0, max_value=5, disable_step_button=True)
st.number_input("Y", min_value=0, max_value=5, disable_step_button=True)

a single TAB gets you to the next input field regardless of the field value.

GitHub Issue Link (if applicable)

This adds on top of #7050 which dynamically hides the buttons to prevent them hiding the input field.

Testing Plan

  • Python tests have been updated to verify default and accept user input
  • JS tests updated to verify default and user specified value

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@jrieke
Copy link
Collaborator

jrieke commented Aug 10, 2023

Hey @DenK007! Thanks for the PR. I thought we had fixed all problems with #7050 but apparently we didn't 😉

I don't really want to introduce a new parameter just for this. Instead, I think we should just change the tabbing behvaior so the +/- buttons get ignored when you tab through the app. I.e. in your example, the focus would jump straight to the next number input. Does that sound like a good solution to you? Should save us from adding a new parameter and also make your life easier since you don't need to set that parameter.

Had a quick look at accessibility and I think from that point of view it's also fine:

  • Default BaseWeb (the underlying framework we use) actually also handles the tabbing behavior as described above, i.e. ignores the stepping buttons. You can try it out here, just set the type as "number", and then tab on the input at the top.
  • There are also other frameworks handling it like this, e.g. see here.
  • The +/- stepping can be done anyway via the up and down arrows on the keyboard. So this widget would still be fully keyboard-accessible if we ignore the step buttons during tabbing. I would even argue that it makes it a bit more accessible since users can more easily navigate to the next input.

If that sounds good to you, feel free to update your PR accordingly and then we can review and merge it!

@jrieke
Copy link
Collaborator

jrieke commented Aug 10, 2023

PS: Note that a bunch of people on the team (incl me) have scheduled vacations during the next few weeks, so might be that we are slow to answer.

@denck007
Copy link
Contributor Author

@jrieke That would solve my problem and probably improve accessibility as well. I rebased and removed the step buttons from the tabIndex. Let me know if I need to do anything else.

Copy link
Collaborator

@vdonato vdonato left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @denck007!

Code-wise this LGTM, but I'll wait for a final confirmation (from either @jrieke or @sfc-gh-jcarroll) on this behavior before clicking the submit button on this PR.

@jrieke
Copy link
Collaborator

jrieke commented Aug 17, 2023

LGTM! @vdonato feel free to merge.

@vdonato vdonato merged commit 979f741 into streamlit:develop Aug 17, 2023
48 checks passed
@denck007 denck007 deleted the number_input_no_button branch August 17, 2023 23:56
eric-skydio pushed a commit to eric-skydio/streamlit that referenced this pull request Dec 20, 2023
zyxue pushed a commit to zyxue/streamlit that referenced this pull request Mar 22, 2024
zyxue pushed a commit to zyxue/streamlit that referenced this pull request Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants