-
Notifications
You must be signed in to change notification settings - Fork 65
feat: Add input_submit_textarea()
input element
#1204
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
base: main
Are you sure you want to change the base?
Conversation
a4344a5
to
693b87c
Compare
7aacb20
to
1d69765
Compare
bde62d7
to
443e2b1
Compare
443e2b1
to
981dd96
Compare
input_submit_textarea()
/input_submit_button()
componentsinput_submit_textarea()
input element
This reverts commit 86eec98.
…e we need it; refactor JS logic
…; refactor/simplify CSS rules
@gadenbuie ready for another review pass when you are |
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.
I'm going to pull changes locally to poke around, but two things jumped out at me from the web view
R/input-submit.R
Outdated
#' @param ... Unnamed argument values: UI elements to include alongside the | ||
#' submit button (e.g., help text, links, etc.). Named argument values: | ||
#' additional attributes to apply to the underlying `<textarea>` element | ||
#' (e.g., spellcheck, autocomplete, etc). |
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.
I have reservations about passing unnamed arguments from ...
into the area next to the button.
For now, I'd rather we only accept named attributes and add them to the text area.
In general, I think this is a place where we shouldn't rely on ...
and should have an argument that takes the UI elements that would go into the tray, or tool bar or whatever we end up calling it. I think that not allowing children in ...
would give us some space to release bslib with input_submit_textarea()
and figure out that API in the future.
My core objection is that I think we should be extremely reluctant to separate the destination of named and unnamed arguments in ...
. I think some separation is inevitable when it makes sense for attributes to be applied at a higher level in the DOM than where children are inserted, but they still need to be conceptually related to each other (things going into the sidebar, for example).
In this case, where attributes go to <textarea>
and children go into some other location, it seems too far away to me. Having an argument for the children destination makes the final solution more readable and understandable because it'll be clear where children end up by the name of the argument.
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.
Good point. I had briefly considered this, but hesitated on committing to name. Anyway, done in 7ccf8e8
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.
One final bit of polish: Currently, you have to click directly on the <textarea>
input to give it focus, but it's easy to click on whitespace that's part of the container, thinking that you'll be clicking into the text area. I think it'd be best if any click inside the .bslib-input-submit-textarea
component that isn't on the submit button would move focus to the text area.
Co-authored-by: Garrick Aden-Buie <[email protected]>
Adds
input_submit_textarea()
, a new input control that's akin toshiny::textAreaInput()
, but delays the sending of an input value until a corresponding button is pressed.Here is a hello world example:
TODO