Add Reset button to restore inital timeline view#155
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds a reset control to the timevis widget, wires a new ChangesReset to Initial View Functionality
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a "Reset to Initial View" button to the timevis widget, allowing users to reset the timeline view to its initial state. The feedback suggests removing a typo in the button's class name (saction-button) and replacing the shiny::icon with a unicode character ("↺") to avoid runtime dependencies in non-Shiny environments. Additionally, it is recommended to update the originalWindow variable on every render instead of only when it is null, ensuring the reset functionality works correctly when the widget is updated with new data.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@inst/htmlwidgets/timevis.js`:
- Around line 253-264: Add a Shiny message handler that maps a new message name
(e.g. 'resetWindow') to the existing resetTimevis JavaScript method so the
timeline can be reset programmatically from R; update the handlers array in
inst/htmlwidgets/timevis.js where other handlers like 'zoomIn' and 'zoomOut' are
registered to call timeline.setWindow via resetTimevis, and add a small R
wrapper function in R/api.R (name it e.g. resetWindowTimevis or resetTimevis)
that sends the matching Shiny message to the widget (mirroring how
zoomIn/zoomOut wrappers are implemented).
- Around line 169-175: The originalWindow is being set immediately after calling
fit, which happens before queued timeline API calls (e.g., setWindow) are
executed, so move the block that assigns originalWindow = { start:
timeline.getWindow().start, end: timeline.getWindow().end } to after the loop
that executes queued API calls (the code that iterates and applies pending API
functions such as setWindow) so the captured window reflects the final
initialized state; locate references to originalWindow, timeline.getWindow(),
fit(), and the loop that processes queued API calls and place the capture
immediately after that loop completes.
In `@R/timevis.R`:
- Line 599: Remove the mistaken "saction-button" CSS class from the zoom-reset
button declaration in R/timevis.R (the element currently rendered with class =
"btn btn-default btn-lg zoom-reset saction-button"); this class is a typo and
unnecessary because the button is wired via onclick in timevis.js (see the
handler referenced around line 43), so update the class string to exclude
"saction-button" (or rename to "action-button" only if you intend to use Shiny
action binding) so the button matches the zoom-in/zoom-out siblings and behaves
correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fd3b5160-5d11-4a88-ac66-69eb4b0100e1
📒 Files selected for processing (2)
R/timevis.Rinst/htmlwidgets/timevis.js
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
R/timevis.R (1)
27-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate
showZoomdoc to mention the reset button.The reset button shares the same
zoom-menu/data-show-zoomtoggle as the zoom in/out buttons (seetimevis.jslines 170-175), soshowZoomnow also controls its visibility, but the docs still only mention "Zoom In"/"Zoom Out".📝 Proposed doc update
-#' `@param` showZoom If \code{TRUE} (default), then include "Zoom In"/"Zoom Out" -#' buttons on the widget. +#' `@param` showZoom If \code{TRUE} (default), then include "Zoom In"/"Zoom Out"/"Reset" +#' buttons on the widget.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@R/timevis.R` around lines 27 - 28, Update the `showZoom` roxygen in `timevis.R` so it explicitly says this flag controls the visibility of the Zoom In, Zoom Out, and Reset buttons. Keep the change limited to the `showZoom` parameter documentation and align the wording with the `timevis.js` zoom-menu behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@inst/htmlwidgets/timevis.js`:
- Around line 180-226: The initial window capture in the timevis init flow is
happening too late, after fit() and queued API calls, so the rangechanged
listener can miss the real starting window; update the init logic around the
timeline fit and the rangechanged registration to capture originalWindow
synchronously or attach the listener before invoking the API queue. Also expand
the hasWindowCall detection in the API loop to treat centerItem, zoomIn, and
zoomOut as window-changing methods so the capture logic handles those cases
correctly.
In `@R/api.R`:
- Around line 638-673: The roxygen example block in resetTimevis is broken
because several example lines use plain # instead of #', which splits the
documentation block and can cause the Rd output to lose the description, params,
and examples. Restore the #' prefixes throughout the example in resetTimevis so
the entire comment stays in one roxygen block and the generated documentation
remains intact.
---
Outside diff comments:
In `@R/timevis.R`:
- Around line 27-28: Update the `showZoom` roxygen in `timevis.R` so it
explicitly says this flag controls the visibility of the Zoom In, Zoom Out, and
Reset buttons. Keep the change limited to the `showZoom` parameter documentation
and align the wording with the `timevis.js` zoom-menu behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ca3988cc-e908-4a41-b682-cd191566a893
📒 Files selected for processing (3)
R/api.RR/timevis.Rinst/htmlwidgets/timevis.js
Problem
While using timevis in a project, we frequently needed to zoom in and out
of the timeline using the mouse wheel to inspect specific time periods.
However, getting back to the initial view was very difficult because:
when the timeline first loaded
This made the user experience frustrating, especially when working with
timelines that span long time periods.
Solution
Added a reset button to the zoom menu that restores the timeline to its
exact initial view (the window state when the timeline first rendered).
Changes made:
timevis_html()renderValue()resetTimevis()in JS to restore the stored windowWhy this is useful
Anyone using timevis with large datasets or long time ranges will benefit
from this — being able to freely explore the timeline by zooming and
panning without worrying about losing the original view makes the widget
much more user friendly.
Testing
Tested in a Shiny app by:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Other