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

Add input_switch() #483

Merged
merged 13 commits into from
Jul 12, 2023
Merged

Add input_switch() #483

merged 13 commits into from
Jul 12, 2023

Conversation

cpsievert
Copy link
Collaborator

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jun 20, 2023

CLA assistant check
All committers have signed the CLA.

@cpsievert cpsievert marked this pull request as ready for review June 26, 2023 23:15
@cpsievert cpsievert assigned cpsievert and gadenbuie and unassigned cpsievert Jun 26, 2023
Copy link
Member

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

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

I left a few small comments and suggestions below.

I wonder if we should include arguments for disabled and reverse (to disable the input and to reverse the order of the label and input). I certainly see a lot of utility for disabled, but I think we'd also want to allow update_switch() to disable the input, which would require us to write some swtich-specific JS in bslib or to update the checkbox input binding in shiny. (I lean toward a bslib-specific approach.)

I'd be happy to pick up this PR and add both arguments.

R/input-switch.R Outdated Show resolved Hide resolved
R/input-switch.R Outdated Show resolved Hide resolved
R/input-switch.R Outdated Show resolved Hide resolved
R/input-switch.R Show resolved Hide resolved
R/input-switch.R Outdated Show resolved Hide resolved
@gadenbuie
Copy link
Member

I wonder if we should include arguments for disabled and reverse (to disable the input and to reverse the order of the label and input). I certainly see a lot of utility for disabled, but I think we'd also want to allow update_switch() to disable the input, which would require us to write some switch-specific JS in bslib or to update the checkbox input binding in shiny. (I lean toward a bslib-specific approach.)

@cpsievert and I discussed in person: this PR will focus on feature parity with Shiny for Python and we'll tackle the above ideas in a future PR

@gadenbuie
Copy link
Member

@cpsievert I applied my suggestions and added a small example app. We could suggest a similar app for Python.

R/input-switch.R Outdated Show resolved Hide resolved
R/input-switch.R Outdated Show resolved Hide resolved
@cpsievert
Copy link
Collaborator Author

@cpsievert
Copy link
Collaborator Author

Let's add a NEWS item then merge away!

@gadenbuie gadenbuie merged commit 1a5e824 into main Jul 12, 2023
10 of 11 checks passed
@gadenbuie gadenbuie deleted the input-switch branch July 12, 2023 16:03
cpsievert added a commit that referenced this pull request Jul 12, 2023
Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>
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.

3 participants