-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
317 log shiny input changes #318
Conversation
Merge branch 'main' of https://github.com/insightsengineering/teal.goshawk # Conflicts: # R/tm_g_gh_boxplot.R
Merge branch 'main' of https://github.com/insightsengineering/teal.goshawk # Conflicts: # R/tm_g_gh_boxplot.R
Code Coverage Summary
Diff against main
Results for commit: d9bd063 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
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 comparing the update that we did here with the previous PR:
https://github.com/insightsengineering/teal.goshawk/pull/298/files
Should we include the log_shiny_input_changes in toggle_slider_server
and srv_arbitrary_lines
too like we did previously?
@donyunardi no, it's not needed. Did you run the app on your own and verified that you see logs or not? I can see logs for the toggle slider You see those changes logged because it is used in other modules, which use |
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 wanted to understand the differences between the two PR updates since they're related, in case I missed something.
I ran a couple of plots and saw the log changes for the sliders and the arbitrary line.
LGTM
Closes #317
Tested with
?tm_g_gh_boxplot
.Output of what was registered in changes