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

textInput(), textAreaInput(), numericInput(), passwordInput(): allow updating value on blur #4183

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@

* Shiny's Typescript assets are now compiled to ES2021 instead of ES5. (#4066)

* When `textInput()` is called with `updateOn="blur"`, the input value will update only when the text input loses focus, or when the user presses Enter, instead of updating as the user types. (#4183)


## Bug fixes

* Fixed a bug with modals where calling `removeModal()` too quickly after `showModal()` would fail to remove the modal if the remove modal message was received while the modal was in the process of being revealed. (#4173)
Expand Down
5 changes: 3 additions & 2 deletions R/input-numeric.R
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@
#'
#' @export
numericInput <- function(inputId, label, value, min = NA, max = NA, step = NA,
width = NULL) {
width = NULL, ..., updateOn = c("input", "blur")) {
updateOn <- match.arg(updateOn)

value <- restoreInput(id = inputId, default = value)

# build input tag
inputTag <- tags$input(id = inputId, type = "number", class="shiny-input-number form-control",
value = formatNoSci(value))
value = formatNoSci(value), `data-update-on` = updateOn)
if (!is.na(min))
inputTag$attribs$min = min
if (!is.na(max))
Expand Down
6 changes: 4 additions & 2 deletions R/input-password.R
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@
#' }
#' @export
passwordInput <- function(inputId, label, value = "", width = NULL,
placeholder = NULL) {
placeholder = NULL, ..., updateOn = c("input", "blur")) {
updateOn <- match.arg(updateOn)

div(class = "form-group shiny-input-container",
style = css(width = validateCssUnit(width)),
shinyInputLabel(inputId, label),
tags$input(id = inputId, type="password", class="shiny-input-password form-control", value=value,
placeholder = placeholder)
placeholder = placeholder, `data-update-on` = updateOn)
)
}
10 changes: 8 additions & 2 deletions R/input-text.R
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
#' @param placeholder A character string giving the user a hint as to what can
#' be entered into the control. Internet Explorer 8 and 9 do not support this
#' option.
#' @param updateOn A character vector specifying when the input should be
#' updated. Options are `"input"` (default) and `"blur"`. If `"blur"`, then
#' the input value will be updated when the text input loses focus, or when
#' Enter is pressed.
#' @return A text input control that can be added to a UI definition.
#'
#' @family input elements
Expand All @@ -35,14 +39,16 @@
#'
#' @export
textInput <- function(inputId, label, value = "", width = NULL,
placeholder = NULL) {
placeholder = NULL, ..., updateOn = c("input", "blur")) {

updateOn <- match.arg(updateOn)

value <- restoreInput(id = inputId, default = value)

div(class = "form-group shiny-input-container",
style = css(width = validateCssUnit(width)),
shinyInputLabel(inputId, label),
tags$input(id = inputId, type="text", class="shiny-input-text form-control", value=value,
placeholder = placeholder)
placeholder = placeholder, `data-update-on` = updateOn)
)
}
4 changes: 3 additions & 1 deletion R/input-textarea.R
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
#'
#' @export
textAreaInput <- function(inputId, label, value = "", width = NULL, height = NULL,
cols = NULL, rows = NULL, placeholder = NULL, resize = NULL) {
cols = NULL, rows = NULL, placeholder = NULL, resize = NULL, ...,
updateOn = c("input", "blur")) {

value <- restoreInput(id = inputId, default = value)

Expand All @@ -67,6 +68,7 @@ textAreaInput <- function(inputId, label, value = "", width = NULL, height = NUL
style = style,
rows = rows,
cols = cols,
`data-update-on` = updateOn,
value
)
)
Expand Down
23 changes: 16 additions & 7 deletions inst/www/shared/shiny.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions inst/www/shared/shiny.js.map

Large diffs are not rendered by default.

24 changes: 12 additions & 12 deletions inst/www/shared/shiny.min.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions inst/www/shared/shiny.min.js.map

Large diffs are not rendered by default.

9 changes: 8 additions & 1 deletion man/numericInput.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 14 additions & 1 deletion man/passwordInput.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion man/textAreaInput.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 14 additions & 1 deletion man/textInput.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 20 additions & 8 deletions srcts/src/bindings/input/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,32 @@ class TextInputBindingBase extends InputBinding {
}

subscribe(el: TextHTMLElement, callback: (x: boolean) => void): void {
$(el).on(
"keyup.textInputBinding input.textInputBinding",
// event: Event
function () {
callback(true);
}
);
$(el).on(
const $el = $(el);
const updateOn = $el.data("update-on") || "input";

if (updateOn === "input") {
$el.on(
"keyup.textInputBinding input.textInputBinding",
// event: Event
function () {
callback(true);
}
);
}

$el.on(
"change.textInputBinding",
// event: Event
function () {
callback(false);
}
);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For NumberInputBinding, this change event can get triggered:

  • programmatically, via updateNumericInput() on the server
  • by clicking on the up/down arrows, or pressing up/down keys on the keyboard.

When we enable the option, we'll want the callback to be invoked when receiving the server message, but not when the up/down arrow event causes it.

Copy link
Member

Choose a reason for hiding this comment

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

I handled this in 0f0e4e5 by adding extra data to the change event emitted by the .receiveMessage() method, whose presence signals that the origin of the event was a server-side message. Then, when updateOn === "blur", we ignore change events that aren't server driven.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One thing that's worth considering is the possibility that someone would trigger a change event on a programmatically via JS, like if they use shinyjs. Requiring fromServer: true could make that stop working.

Copy link
Member

Choose a reason for hiding this comment

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

Good point; I'll think about this some more

Copy link
Member

Choose a reason for hiding this comment

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

Oh! I came up with an elegant solution (in d322976): rather than using the event data, we'll just ignore any change events that happen when the element has focus. It's unlikely that programmatic changes will happen while an element has focus, so in most cases the update is immediate. But if the changed input currently has focus, we'll just update on blur like we otherwise would.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh great! That’s much nicer than the other solutions.


if (updateOn === "blur") {
$el.on("blur.textInputBinding", function () {
callback(false);
});
}
}
unsubscribe(el: TextHTMLElement): void {
$(el).off(".textInputBinding");
Expand Down