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

Fix sporadic dates (#3664) #3665

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

dvg-p4
Copy link
Contributor

@dvg-p4 dvg-p4 commented Jul 7, 2022

This pull request removes the handlers for keyup and input from dateInput and dateRangeInput. This prevents spurious updates while typing, but still sends the parser's best interpretation of the field when enter is pressed, focus is lost (i.e. the user clicks out), or a date selection is made in the GUI (due to the remaining changeDate and change handlers).

A value of null (for dateInput) or NA (for dateRangeInput) is still sent immediately when the field is cleared, but this can be more easily checked for and ignored on the server side. I suspect that bootstrap-datepicker is interpreting the empty string as a special value and firing off the changeDate event, and stopping that would probably be much more intensive than commenting out a few lines of code.

This therefore mostly fixes #3664 .

Not sure if y'all will want to shove this into 1.7.2, leave it for the next release, or if there's some crazy use case I can't imagine where you'd want "2001-01-0" to be immediately interpreted as the last day of 1999.

Edit: Merged in 1.7.2-1.7.4 changes, no conflicts except for minified JS as usual

This prevents spurious updates while typing, but still sends when enter is pressed, focus is lost, or the GUI is clicked (due to the remaining `changeDate` and `change` handlers).
@dvg-p4
Copy link
Contributor Author

dvg-p4 commented Aug 12, 2022

On further investigation, it seems that the problem was my own darn fault. I needed to update my roxygen2 from 7.2.0 to 7.2.1, so that devtools::check() would actually redocument the package, fixing the version number in package.json, so that the github actions wouldn't try to do that itself, get mad at itself for doing it, and fail the check. Probably.

@dvg-p4
Copy link
Contributor Author

dvg-p4 commented Aug 12, 2022

...nope, looks like even with updated roxygen, devtools::check() still doesn't update package.json; I need to manually run source .github/shiny-workflows/routine.sh. Or maybe I could have just manually updated the package version. Edit: yeah, just doing that manually also works.

@dvg-p4
Copy link
Contributor Author

dvg-p4 commented Aug 12, 2022

@wch whether or not y'all accept this PR you should probably update the package.json version number on main
edit: cool, looks like that's fixed

Conflicts:
	NEWS.md
	inst/www/shared/shiny.js
	inst/www/shared/shiny.js.map
	inst/www/shared/shiny.min.js
	inst/www/shared/shiny.min.js.map
@dvg-p4
Copy link
Contributor Author

dvg-p4 commented Mar 15, 2023

This will probably fail the R 3.5 test for the same reason as #3683 (comment) and main

@dvg-p4
Copy link
Contributor Author

dvg-p4 commented Mar 15, 2023

It looks like some other checks also failed due to R 4.2.3 being released today but not quite making it to the expected repositories yet

@dvg-p4
Copy link
Contributor Author

dvg-p4 commented Mar 26, 2023

Is there a way to rerun the tests here?

Edit: Never mind, looks like the merge of main did it.

@CLAassistant
Copy link

CLAassistant commented Nov 8, 2023

CLA assistant check
All committers have signed the CLA.

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.

dateInput and dateRangeInput send spurious values to server mid-typing
2 participants