getTicks: Refactor to simplify and reuse functionality between the implementation of getTicksStart and getTicksEnd #3769
Conversation
nikolasrieble
changed the title
src/cartesian/CartesianAxis.tsx
Outdated
| @@ -37,11 +46,10 @@ export interface CartesianAxisProps { | |||
| hide?: boolean; | |||
| label?: any; | |||
|
|
|||
| minTickGap?: number; | |||
| minTickGap?: TickGap; | |||
There was a problem hiding this comment.
why TickGap type is introduced? so far I see it's only alias for number and when you will hit suggestion prompt in any IDE imho (personal preference) it's easier to wrap your head around when it just hint you that is a number instead some kind of custom type.
even if there are plans to expand it like for example:
TickGap = number | string or anything like that - I still would like this notation over a custom type, that I don't know and have to see it's definition first to understand it.
But maybe there is some specific use case that I don't know about and this solves some other issue?
There was a problem hiding this comment.
No, good point. I thought this would help storybook pick up comments above the Type from a single place, but it turned out to not work 🤦
Will remove the alias.
There was a problem hiding this comment.
LGTM 🚀
Even though I added lots of comments 99% of it is non-blocking as this is totally the right direction. I think we should try to limit parameters to our functions somehow - after 4 or 5 it gets difficult to keep track of what is what. Maybe moving to object parameters could help that so at least they must be named when calling? idk
src/util/TickUtils.ts
Outdated
| tickFormatter: TickFormatter, | ||
| entry: CartesianTickItem, | ||
| index: number, | ||
| sizeKey: 'width' | 'height', | ||
| fontSize: string | number, | ||
| letterSpacing: string | number, | ||
| unitSize: Size, | ||
| angle: number, |
There was a problem hiding this comment.
nit: maybe move this input to an object since its getting so busy here for such a small function?
|
🚀 |
…plementation of getTicksStart and getTicksEnd (recharts#3769) <!--- Provide a general summary of your changes in the Title above --> ## Description Refactors getTicks.ts to reuse logic which is shared between getTicksStart and getTicksEnd. The goal of this refatoring is to make it easier to add new methods to compute the ticks, i.e. equidistant ticks. Improvements to TickPositioning story: - Resize (important to test how different interval options lead to ticks disappearing) - Add the interval option under the axis as a title Additionally comments and new types wherever useful. <!--- Describe your changes in detail --> ## Related Issue recharts#3305 <!--- This project only accepts pull requests related to open issues --> <!--- If suggesting a new feature or change, please discuss it in an issue first --> <!--- If fixing a bug, there should be an issue describing it with steps to reproduce --> <!--- Please link to the issue here: --> ## Motivation and Context <!--- Why is this change required? What problem does it solve? --> ## How Has This Been Tested? <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## Screenshots (if appropriate): ## Types of changes <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] My code follows the code style of this project. - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [ ] I have added tests to cover my changes. - [x] I have added a storybook story or extended an existing story to show my changes - [x] All new and existing tests passed. --------- Co-authored-by: “Nikolas <“nikolas@rieble.com“>
Description
Refactors getTicks.ts to reuse logic which is shared between getTicksStart and getTicksEnd. The goal of this refatoring is to make it easier to add new methods to compute the ticks, i.e. equidistant ticks.
Improvements to TickPositioning story:
Additionally comments and new types wherever useful.
Related Issue
#3305
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: