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.

Sign up for GitHub

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

Jump to bottom

feat(axis): support object type tickLabelProp #1662

Merged
merged 3 commits into from Mar 8, 2023
Merged

feat(axis): support object type tickLabelProp #1662

merged 3 commits into from Mar 8, 2023

Conversation

Robin-Hoodie
Copy link
Contributor

🚀 Enhancements

  • Allow specifying tickLabelProps as an object
  • Keep non-overridden default props when specifying tickLabelProps as an object

Resolves #1657

@Robin-Hoodie
Allow specifying 'tickLabelProps' as object
d63ee17 
In addition, all places where a function was used without using any
of the arguments supplied to the function, I replaced these by an object as
this should be slightly more efficient/readable
@Robin-Hoodie
Don't have consumer override all default props if only specifying a c…
7b437c3 
…ouple of them

Use default tick label props and override just the props that the
consumer may have specified. This only works if they specify the props
as an object, not as a function
@Robin-Hoodie Robin-Hoodie changed the title Tick label props @visx/axis: Keep non-overridden default tick label props Mar 1, 2023
@Robin-Hoodie Robin-Hoodie changed the title @visx/axis: Keep non-overridden default tick label props feat(@visx/axis): Keep non-overridden default tick label props Mar 1, 2023
williaster
williaster reviewed Mar 6, 2023
View reviewed changes
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.

@Robin-Hoodie thanks for the awesome addition! this will definitely clean up this API / give a better dev experience. 🙌

Overall looks good to me but I had one suggestion that might make it a bit more readable/reduce complexity.

packages/visx-axis/src/axis/AxisBottom.tsx Show resolved Hide resolved
packages/visx-axis/src/utils/getTickLabelProps.ts Outdated Show resolved Hide resolved
@williaster williaster changed the title feat(@visx/axis): Keep non-overridden default tick label props feat(axis): support object type tickLabelProp Mar 6, 2023
williaster
williaster approved these changes Mar 7, 2023
View reviewed changes
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.

lgtm! thanks again @Robin-Hoodie 💯

packages/visx-axis/src/axis/AxisLeft.tsx Show resolved Hide resolved
@williaster williaster merged commit e6b24f7 into airbnb:master Mar 8, 2023
@github-actions
Copy link

github-actions bot commented Mar 8, 2023

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

@Robin-Hoodie Robin-Hoodie deleted the tick-label-props branch March 8, 2023 08:42
@williaster williaster mentioned this pull request Jan 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@williaster williaster williaster approved these changes

Assignees
No one assigned
Labels
🚀enhancement minor-release @visx/axis @visx/react-spring @visx/xychart
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Specifying tick label props should not wipe all defaults
2 participants
@Robin-Hoodie @williaster