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

Fixup categoryorder for missing values in cartesian traces #5268

Merged
merged 2 commits into from Nov 12, 2020

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Nov 12, 2020

Fixes #5267.

@plotly/plotly_js
cc: @cldougl

@nicolaskruchten
Copy link
Member

@antoinerg can you take a look here as well plz, given that you did the original implementation here?

The original reports were strangely hard to reproduce: #5267 ... does your fix sort of suggest why this might be machine/browser/environment-dependent, @archmoj ?

@archmoj
Copy link
Contributor Author

archmoj commented Nov 12, 2020

The original reports were strangely hard to reproduce: #5267 ... does your fix sort of suggest why this might be machine/browser/environment-dependent, @archmoj ?

Possibly related to the fact that the width and height were not set on the mock you were testing...

@nicolaskruchten
Copy link
Member

Ah that's an interesting possibility... But why does the category order depend on the width?!?

@archmoj
Copy link
Contributor Author

archmoj commented Nov 12, 2020

Ah that's an interesting possibility... But why does the category order depend on the width?!?

I've not noticed this bug on different browsers.
Anyway the width may impact the bin size for histograms maybe?

@nicolaskruchten
Copy link
Member

This is a categorical histogram so there is no bin size :)

@antoinerg
Copy link
Contributor

@archmoj can you point me where in the code the existing behavior was changed.

@archmoj
Copy link
Contributor Author

archmoj commented Nov 12, 2020

@nicolaskruchten
Testing this mock on different OS+browsers leads to the identical error as far as I could test :)

@antoinerg
Copy link
Contributor

Thanks @archmoj for the nice code cleanup and bug fix! LGTM 💃

@archmoj archmoj merged commit a51b91f into master Nov 12, 2020
@archmoj archmoj deleted the fix5267-cartesian-category-values branch November 12, 2020 21:39
@archmoj archmoj added this to the v1.58.0 milestone Nov 22, 2020
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.

category order bug
3 participants