Implement layout.barbase (closes issue 475) #2
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
* Simplified algorithm to identify overlapping bars. * Removed the closure setBarCenter. * Checked that jasmine tests still pass.
* Converted the closure that groups vertical and horizontal bars into a function. * This change will help split setPositions into functions for each barmode.
* Moved code to group bars from setGroupPositions to setPositions.
* Converted closure `barpositions` into function `setOffsetAndWidth`. * This change will help split setPositions into functions for each barmode.
* Don't assume the position axis is `xa` in the calculation of `barDelta` in `bar/hover.js`. * Updated `tests/bar_test.js` to account for the replacement of `t.bar` with `t.bargroupwidth`. * Updated the group case in `tests/bar_test.js` to test the use of `layout.bargroupgap`.
* Refactored the code to set bar positions in overlay mode into function `setGroupPositionsInOverlayMode`.
* Refactored the code for setting bar positions into setGroupPositionsInOverlayMode, setGroupPositionsInGroupMode and setGroupPositionsInStackOrRelativeMode. * Refactored code for stacking bars and computing minDiff into helper class Sieve.
* Renamed setBaseAndSize to stackBars. * Refactor code to normalize bars into function normalizeBars.
* Fix: do not normalise bars with no size * Fix: update minDtick if barmode is group and bars overlap. Closes plotly#475
|
@n-riesco your refactoring commits are looking good. 🎉 Thanks! I apologise again for the I'm a little concerned about merging this in though as Thanks again for your hard work. |
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.
I'm working on issues plotly#475 and plotly#80 . Both require changes in
set_positions.js. For this reason, this PR not only implementslayout.barbase(issue plotly#475), but also refactors the code inset_positions.jsinto functions for eachlayout.barmode. This change will help me with the next PR to implement issue plotly#80.I think it's best to review each commit separately.