Skip to content
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

Mult axis formatting #6334

Merged
merged 63 commits into from
Dec 22, 2022
Merged

Mult axis formatting #6334

merged 63 commits into from
Dec 22, 2022

Conversation

hannahker
Copy link
Contributor

@hannahker hannahker commented Oct 3, 2022

Work in progress changes to enable automatic formatting of multiple y axes on plots. In rough order of priority, to dos include:

  • TEST: Write new Jasmine tests to check that ax._shifts are set correctly
  • FEATURE Properly account for title standoff in full axis size
  • FEATURE: Shift by drawn axis size rather than constant
  • BUG: Redraw axes when config: {scrollZoom: true}
  • BUG: Respect axis dragging and moving
  • TEST: Fix currently failing Jasmine test for zeroline
  • TEST: Write new Jasmine tests to check that setting defaults is done correctly
  • FEATURE: Handle case where shift is given a numerical input
  • TEST: Fix currently failing tests due to invalid yaxis.shift=true (mock validation and bundle jasmine)
  • If yaxis.shift=true, set default position according to domain of counter axis
  • Draw axis line correctly (showline=true)
  • Increment and apply shift value based on axes of the same overlaying group
  • Increment and apply constant 60px shift value in cases with multiple axes
  • Adjust automargin to account for shifted axis width
  • BUG: Fix drawing of axis lines when x-axis domain is manually specified and automargin push does not need to be applied
  • Fix automargin on inset axes when tick label length increases
  • Fix overlap with legend [out of scope - separate PR]
  • Automargin correctly with subplot columns [out of scope - will not be addressed]

src/plots/cartesian/axes.js Outdated Show resolved Hide resolved
@hannahker
Copy link
Contributor Author

@archmoj - I'm still struggling to figure out how the mock validation is failing here with the error: "In layout, key yaxis5.shift is set to an invalid value (true)".

I've added the extras: [true, false] param to the cartesian/layout_attributes.py but am still getting this error. Do you know what the cause for this might be?

@@ -10735,6 +10735,12 @@
"editType": "ticks",
"valType": "boolean"
},
"shift": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one appears to be on the xaxis.
Yet we are not going to implement this feature for the x axes in this PR.
If that's the case, let's make the adjustments in the attribute file so that it would only be added to the yaxis .
Thanks!

src/plots/plots.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 Fantastic. Great work @hannahker in a really tricky part of our code! Only remaining piece that I see is getting the new attrs out of the x axis schema - and I suppose adding a changelog entry.

src/plots/plots.js Outdated Show resolved Hide resolved
@archmoj
Copy link
Contributor

archmoj commented Dec 21, 2022

Great work! Thanks @hannahker 🎖️ 🏅 🥇 🏆 🙏 for the contribution!
💃 after making the draftlogs/6334_add.md file 🙂

@hannahker hannahker merged commit 4964952 into master Dec 22, 2022
@hannahker hannahker deleted the mult-axis-formatting branch December 22, 2022 19:11
@hannahker hannahker mentioned this pull request Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants