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

Position title after legend width calculated #6725

Merged
merged 11 commits into from Sep 22, 2023

Conversation

28raining
Copy link
Contributor

To continue fixing #6668 (comment)

The original method worked because the legend-draw function is called multiple times. I don't know why, it seems like a mistake. On the 2nd call, the legend has been drawn and legendObj._width has been calculated.

Sometimes the function is only called once. I was able to reproduce by changing the height of one of the mocks. This mean legend horizontal align didn't work.

I've updated the code to re-draw the legend title after computeLegendDimensions

  • the first draw allows the title height to be accounted for in computeLegendDimensions
  • the second draw just has purpose of horizontal aligning the title

@28raining
Copy link
Contributor Author

I've bundled a couple of other issue fixes in here

#6661

  • single-point histograms have an existing special workaround to calculate the bin width. But that shouldn't run if the user has defined the bin widths manually, or assigned a bin-group
  • The single-point box is wider than the multi-point box, you can see in my new mock. That's because May is longer than June. Should bars be as wide as the month? Or should all bars be the same width. I think it doesn't look too bad and is a rare corner case, so I'm happy to leave it.

#6685

  • I did file a PR on color-parse, and they recently released v2.0.0. Which I assume includes the .toLowerCase() fix

@28raining
Copy link
Contributor Author

I'm stumped, the tests pass on my personal dockers, and regenerating the images isn't helping

@archmoj
Copy link
Contributor

archmoj commented Sep 18, 2023

I'm stumped, the tests pass on my personal dockers, and regenerating the images isn't helping

There is a bug in our image test system depending on the rendering order.
To avoid that please rename your new mock and baseline to run in the end of process e.g. zz-histogram_overlay-1point-date. You also need to revert the other baseline changes to have a successful run.
Thank you!

package.json Outdated Show resolved Hide resolved
@archmoj
Copy link
Contributor

archmoj commented Sep 19, 2023

Please write draftlog/6725_fix.md file mentioning two different fixes and this should be good to go.
Thank you! 🏅 🏆

@archmoj
Copy link
Contributor

archmoj commented Sep 22, 2023

💃

@archmoj archmoj merged commit 6825695 into plotly:master Sep 22, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants