Fix update_axes to respect selector duration instead of hardcoded 1000ms #277
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tdhock
requested changes
Nov 20, 2025
- Modified update_axes() to accept v_name parameter - Changed duration from hardcoded 1000ms to Selectors[v_name].duration - Defaults to 0ms (no transition) when duration not specified - Added test to verify 2000ms custom duration is respected
tdhock
requested changes
Nov 24, 2025
Collaborator
tdhock
left a comment
tdhock
left a comment
There was a problem hiding this comment.
please fix test failures before asking for review
- Changed condition from 'Selectors[v_name].duration' to 'hasOwnProperty("duration")'
- Previous check failed when duration explicitly set to 0 (falsy value)
- Now properly distinguishes between 'duration not specified' vs 'duration=0'
- Updated test to use factor selector for reliable axis domain changes
Added typeof check to ensure duration is a valid number before using it. This prevents issues if duration is set to undefined, null, or other types.
Test now properly creates axis domains that change between years.
tdhock
reviewed
Nov 25, 2025
tdhock
reviewed
Nov 25, 2025
JavaScript: Use simple '|| 0' instead of multiple checks Test: Check tick changes instead of axis transform positions
The test was passing the entire HTML document to getTickDiff() instead of extracting the axis node first using getNodeSet(). This follows the pattern used in test-renderer3-update-axes.R.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #277 +/- ##
==========================================
+ Coverage 69.53% 73.08% +3.55%
==========================================
Files 163 164 +1
Lines 6004 8766 +2762
==========================================
+ Hits 4175 6407 +2232
- Misses 1829 2359 +530
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Sir @tdhock Can you please review this PR now and give your feedback is there anything I need to change . |
Contributor
Author
tdhock
requested changes
Nov 26, 2025
tdhock
requested changes
Nov 27, 2025
tdhock
requested changes
Nov 27, 2025
Contributor
Author
|
Replaced magic numbers with unequal() function using tolerance parameter, following the same pattern as [test-renderer3-update-axes. |
tdhock
requested changes
Nov 28, 2025
Contributor
Author
|
I sincerely apologize Sir you are absolutely right I should have followed your previous feedback
|
tdhock
requested changes
Dec 3, 2025
ANAMASGARD
force-pushed
the
fix-update-axes-duration
branch
from
38b2082 to
8e9d103
Compare
…ctor - Pass v_name parameter to update_axes() to access selector duration - Use selector's duration instead of hardcoded 1000ms - Fix selector to use Plots[p_name].plot_id instead of manual construction - Add null check for v_name before accessing Selectors
…ctor - Pass v_name parameter to update_axes() to access selector duration - Use selector's duration instead of hardcoded 1000ms - Fix selector to use Plots[p_name].plot_id instead of manual construction - Add null check for v_name before accessing Selectors - Fix test-compiler-ghpages.R to exclude common chunk TSV files from count
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #276
Problem
The
update_axes()function in the JavaScript renderer was using a hardcoded transition duration of 1000ms, regardless of what duration was set for the selector triggering the axis update.This meant that when a user specified
duration=list(year=2000)for a year selector, clicking to change years would cause:Testing
Added
test-renderer3-update-axes-duration.Rwhich:duration=list(year=2000)This failing test committed first, then the fix in a upcoming commits .