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

new(axis): add shapeRendering prop to Axis #1739

Merged
merged 1 commit into from
Mar 4, 2024
Merged

new(axis): add shapeRendering prop to Axis #1739

merged 1 commit into from
Mar 4, 2024

Conversation

Lonami
Copy link
Contributor

@Lonami Lonami commented Aug 4, 2023

💥 Breaking Changes

None.

🚀 Enhancements

Additional flexibility when using Axis.

📝 Documentation

shapeRendering can now be configured for Axis lines.

🐛 Bug Fix

Not sure if this was introduced by #840. I haven't dug much into it. But I have noticed that on Windows, Firefox, when using the Axis with a scaleLinear and dashed pattern, some vertical lines looked very thick while others very thin.

After playing around in the HTML, I figured the culprit was the "smart" shapeRendering. So I want a way to disable it without resorting to ugly hacks.

🏠 Internal

The CommonProps for visx-types have changed.

@Lonami
Copy link
Contributor Author

Lonami commented Mar 3, 2024

Hey, this has been sitting around for a while. Could I get a review or some insight on what needs to be done to have pull requests merged @williaster ? Is it the PR description? Thanks!

Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

Hey @Lonami, thanks for the ping and sorry for the delay. this lgtm! appreciate the addition for everyone! Should be released shortly.

@williaster williaster changed the title Add shapeRendering prop to Axis new(axis): add shapeRendering prop to Axis Mar 4, 2024
@williaster williaster merged commit 3e1a3ef into airbnb:master Mar 4, 2024
Copy link

github-actions bot commented Mar 4, 2024

🎉 This PR is included in version v3.9.0 of the packages modified 🎉

@Lonami
Copy link
Contributor Author

Lonami commented Mar 5, 2024

Hey @williaster, sorry I just tried this and it turns out it's not doing anything. The prop added here is pretty much dead because what gets passed down to the line is tickLineProps

…but this has been released now, so I guess removing it would be a breaking change?

@williaster
Copy link
Collaborator

Oh boy, so was possible before by adding to tickLineProps, sorry I didn't catch that 👍

not a huge deal, I'll PR to revert and we can just do a new release. I wouldn't consider it breaking since it wasn't functional.

williaster added a commit that referenced this pull request Mar 8, 2024
williaster added a commit that referenced this pull request Mar 8, 2024
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.

2 participants