-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Properly handle undefined value for input subscribe callback #4243
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
Conversation
f4cda6c
to
846c7cd
Compare
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.
Looks good! I just have one small refactoring comment now that I'm looking at this again
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.
This fixes issues I have with 1.11.0 (other than the issue from #4242). Can you add a test so this bug doesn't make it to CRAN again? It looks like {shinyWidgets} also lacks a lot of tests, so that's how this made it through without revdeps catching it.
* main: (32 commits) Revert the addition of spacing between `icon` and `label` in `actionButton()` (#4248) Restrict `icon`/`label` separator spacing to `actionButton()` (#4247) Fix front-end action button label updating logic (#4242) Update news Properly handle undefined value for input subscribe callback (#4243) Start new version (#4241) v1.11.0 release candidate (#4232) Follow up to #3996: fix front-end checkbox label updating logic (#4238) feat(InputBinding): subscribe callback now supports event priority (#4211) Follow up to #3996 when label is unspecified (i.e., NULL), don't include it in the message (#4237) Run routine (#4234) chore: #4175 update jquery-ui to 1.14.1 (#4205) Update jQuery to 3.7.1 (#3969) Fix 404 in example 08_html (shiny.min.css) (#4221) Follow up to #3870: fix location of news item (#4233) Bugfix for error found in tests (#3870) Allow update input labels with HTML (#3996) Adds mirai to documentation (#4230) family->given for R Core authorship (#4222) fix(renderPlot): get interactive plotting working with ggplot2 v4.0 (#4228) ...
Closes #4240
This regression was introduced by #4211, where support for new values for added to
InputBinding.subscribe()
'scallback
. There, I overlooked the fact that we did support no value (i.e.,undefined
) passed to the.callback()
, but the new code wouldn't handle it properly.Now we properly handle an
undefined
value (and also type that case properly)