You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains 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
Addresses the issue where bar chart items more than 50% out of their chart area when zoomed disappear completely, as the bar middle point was being used to determine whether they should be culled from view.
@carbonrobot this fixes the issue in 2905, in a way that I don't think is going to conflict with anything else. There's a lot of jumping around in logic, from the actual component class, to its helper methods, into data, and into domain as well. I don't think this the best way to fix it in terms of code maintainability and understandability, but I think to have a better fix would require a larger refactor, which seems like something coming in the future. There might also be a slightly cleaner check than component.type.displayName, I'll take a look into that refactor once I get your thoughts and see if you'd definitely like to go this route.
After seeing @nlkluth's solution here, I gave a try at subclassing VictoryBar and overriding renderData, getBaseProps and getCalculatedValues from the EventsMixin in the derived class to see if this could be accomplished without modifying internal packages (aside from exposing an export or 2). Unfortunately, the overrides got out of control, each additional one triggering an error that required another. I think what's here might be the best way to go at this point.
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.
nstolpe
force-pushed
the
fix/bars-disappear-on-zoom
branch
from
Description
Addresses the issue where bar chart items more than 50% out of their chart area when zoomed disappear completely, as the bar middle point was being used to determine whether they should be culled from view.
Fixes # 2905
Type of Change
How Has This Been Tested?
Checklist: (Feel free to delete this section upon completion)
Original video from issue:
372569985-a4f28103-efec-4b80-8af0-52fc8f809142.mov
Video after fix:
Screen.Recording.2024-11-18.at.10.00.56.AM.mov