feat: create options for tick rotation #770

hlyang397 · 2020-08-27T07:57:48Z

User may like to control if ticks need to be rotated ~~while zoom domain is changing~~ .
For example, if the tick number is small and the chart is wide enough, maybe tick rotation is not necessary.

Updates

new enums and configuration
control x-axis tick rotation depending on configuration

Demo screenshot or recording

Always rotate ticks ~~while zooming~~:

Never rotate ticks ~~while zooming~~:

Rotate ticks when space is not enough: (default)

Review checklist (for reviewers only)

- new enums and configuration - control x-axis tick rotation depending on configuration

packages/core/src/components/axes/axis.ts

theiliad · 2020-08-27T13:50:55Z

packages/core/src/components/axes/axis.ts

@@ -472,8 +511,7 @@ export class Axis extends Component {
 					: estimatedTickSize < minTickSize;
 			}

-			// always rotate ticks if zoomDomain is changing to avoid rotation flips during zoomDomain changing
-			if (rotateTicks || this.zoomDomainChanging) {
+			if (this.isTickRotationRequired(rotateTicks)) {


why would we go through labels and calculate whether any need rotation before knowing whether we are rotating or not?

You are right, if we already know that we need to rotate or not, we could avoid some calculation

Updated in f2489e7.

theiliad

Couple of points here:

the code here strictly works for bottom & top axis. It should just really factor in which zoom bars are active and act accordingly
we should probably fix the issue of unnecessary tick rotations before fixing this. Seems like ticks rotate at times when they don't need to

…only

…s into label-rotation-options

hlyang397 · 2020-09-02T03:38:49Z

@theiliad,

Couple of points here:

* the code here strictly works for bottom & top axis. It should just really factor in which zoom bars are active and act accordingly

Updated in bc124bb.

* we should probably fix the issue of unnecessary tick rotations before fixing this. Seems like ticks rotate at times when they don't need to

I don't think so.
Even the tick rotation function works perfectly without unnecessary tick rotations, we still need this option to avoid the tick rotation flips during zoom domain changing.
So I think unnecessary tick rotations is another issue, but not necessary a dependency of this issue.

theiliad

again, this seems like it's completely dependant on zooming. Why wouldn't this just work without the zoom bar?

theiliad · 2020-09-02T16:14:02Z

we should just move this flag out of the zoom bar options & into each individual axis:

{
  axes: {
    bottom: {
       ticks: {
           rotation: .....
       }
    }
  }
}

hlyang397 · 2020-09-03T03:58:51Z

again, this seems like it's completely dependant on zooming. Why wouldn't this just work without the zoom bar?

@theiliad ,
Yes, it's completely dependent on zooming.
I guess you misunderstand this option, so I just update the PR title.
This PR is about the tick rotation while zooming, not the tick rotation without zoom bar enabled.
When zoom bar is disabled, tick rotation behavior keeps as it is. It rotates whenever necessary. This option is meaningless.
When zoom bar is enabled, and user is changing zoom domain, this option allows user to decide if

ALWAYS keep tick rotation when zoom domain is changing
NEVER let tick rotated (keep flat) when zoom domain is changing
AUTO let tick rotate whenever needed when zoom domain is changing

That's why I set the flag name rotateWhileZooming, and it has to be related to zoom bar.

If you are thinking about the tick rotation option when zoom domain is NOT changing or zoom bar is disabled, that would be another story.

Hope I make it clear after these explanation.

hlyang397 · 2020-09-03T04:03:34Z

we should just move this flag out of the zoom bar options & into each individual axis:
{
  axes: {
    bottom: {
       ticks: {
           rotation: .....
       }
    }
  }
}

The flag is already under ticks of individual axis, but the name is rotateWhileZooming for now.
As I mentioned in another comment, this option is about tick rotation while zooming, so the name rotation may confuse user.
It's up to you, let me know the flag name you like to use.

theiliad

This really should work without being dependant on zooming.

If you've set the config to auto then it'll result in the library deciding when to rotate and when not to.

If you've set it to never then obviously ticks will never rotate.

If you've set it to always, then they'll always be rotated...

Therefore it should just be a axisOptions.ticks.rotation flag rather than axisOptions.ticks. rotateWhileZooming

hlyang397 · 2020-09-11T08:32:26Z

This really should work without being dependant on zooming.

If you've set the config to auto then it'll result in the library deciding when to rotate and when not to.

If you've set it to never then obviously ticks will never rotate.

If you've set it to always, then they'll always be rotated...

Therefore it should just be a axisOptions.ticks.rotation flag rather than axisOptions.ticks. rotateWhileZooming
@theiliad
The tick rotation option not related to zooming is actually another kind of option.
But this option also works to me for now. Updated in e83c395.

Since this PR is not related to zoom now, I also update PR title and description.

Let me know if you have any comment.

…to linearGradient * 'master' of https://github.com/JennChao/carbon-charts: v0.37.0 Merge pull request carbon-design-system#780 from natashadecoste/fix-meter-max-val feat: enable or disable scatter dot on charts except scatter chart (carbon-design-system#769) feat: create options for tick rotation (carbon-design-system#770)

…to axis-option * 'master' of https://github.com/JennChao/carbon-charts: v0.37.0 Merge pull request carbon-design-system#780 from natashadecoste/fix-meter-max-val feat: enable or disable scatter dot on charts except scatter chart (carbon-design-system#769) feat: create options for tick rotation (carbon-design-system#770) fix: enable zoom in functionality while axis is disabled

…to ruler-option * 'master' of https://github.com/JennChao/carbon-charts: v0.37.1 Merge pull request carbon-design-system#800 from metonym/fix-svelte-base-chart v0.37.0 Merge pull request carbon-design-system#780 from natashadecoste/fix-meter-max-val feat: enable or disable scatter dot on charts except scatter chart (carbon-design-system#769) feat: create options for tick rotation (carbon-design-system#770)

…to tooltip-option * 'master' of https://github.com/JennChao/carbon-charts: v0.37.1 Merge pull request carbon-design-system#800 from metonym/fix-svelte-base-chart v0.37.0 Merge pull request carbon-design-system#780 from natashadecoste/fix-meter-max-val feat: enable or disable scatter dot on charts except scatter chart (carbon-design-system#769) feat: create options for tick rotation (carbon-design-system#770) v0.36.4 Merge pull request carbon-design-system#761 from JennChao/sparkline Merge pull request carbon-design-system#793 from hlyang397/update-initial-zoom-domain fix the defect of label tooltip not showing when using custom tooltip (carbon-design-system#787) v0.36.3 Merge pull request carbon-design-system#785 from sophiiae/skeleton-with-data

* feat: create options for tick rotation - new enums and configuration - control x-axis tick rotation depending on configuration * refactor: change TickRotations DEPENDING to AUTO * refactor: avoid extra calculation for shouldRotateTicks * refactor: limit top/bottom axis tick rotation to top/bottom zoom bar only * refactor: tick rotation don't depend on zoom

feat: create options for tick rotation

c886048

- new enums and configuration - control x-axis tick rotation depending on configuration

hlyang397 requested review from natashadecoste, theiliad and a team as code owners August 27, 2020 07:57

hlyang397 requested review from Donisius and removed request for a team August 27, 2020 07:57

theiliad requested changes Aug 27, 2020

View reviewed changes

hlyang397 added 2 commits August 28, 2020 11:43

refactor: change TickRotations DEPENDING to AUTO

b08826d

refactor: avoid extra calculation for shouldRotateTicks

f2489e7

hlyang397 requested a review from theiliad August 28, 2020 04:53

theiliad requested changes Sep 1, 2020

View reviewed changes

hlyang397 added 2 commits September 2, 2020 11:23

refactor: limit top/bottom axis tick rotation to top/bottom zoom bar …

bc124bb

…only

Merge branch 'master' of github.com:carbon-design-system/carbon-chart…

6741bde

…s into label-rotation-options

theiliad requested changes Sep 2, 2020

View reviewed changes

hlyang397 changed the title ~~feat: create options for tick rotation~~ feat: create options for tick rotation while zooming Sep 3, 2020

hlyang397 requested a review from theiliad September 3, 2020 04:05

theiliad requested changes Sep 10, 2020

View reviewed changes

hlyang397 added 2 commits September 11, 2020 15:54

Merge branch 'master' into label-rotation-options

8bdc0ab

refactor: tick rotation don't depend on zoom

e83c395

hlyang397 changed the title ~~feat: create options for tick rotation while zooming~~ feat: create options for tick rotation Sep 11, 2020

hlyang397 requested a review from theiliad September 11, 2020 08:32

theiliad approved these changes Sep 11, 2020

View reviewed changes

sophiiae approved these changes Sep 11, 2020

View reviewed changes

natashadecoste approved these changes Sep 11, 2020

View reviewed changes

theiliad added the type: enhancement 💡 label Sep 11, 2020

theiliad merged commit 4063c8a into carbon-design-system:master Sep 11, 2020

theiliad deleted the label-rotation-options branch September 11, 2020 16:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: create options for tick rotation #770

feat: create options for tick rotation #770

hlyang397 commented Aug 27, 2020 •

edited

Loading

theiliad Aug 27, 2020

hlyang397 Aug 28, 2020

theiliad left a comment

hlyang397 commented Sep 2, 2020

theiliad left a comment

theiliad commented Sep 2, 2020

hlyang397 commented Sep 3, 2020

hlyang397 commented Sep 3, 2020

theiliad left a comment

hlyang397 commented Sep 11, 2020

feat: create options for tick rotation #770

feat: create options for tick rotation #770

Conversation

hlyang397 commented Aug 27, 2020 • edited Loading

Updates

Demo screenshot or recording

Review checklist (for reviewers only)

theiliad Aug 27, 2020

Choose a reason for hiding this comment

hlyang397 Aug 28, 2020

Choose a reason for hiding this comment

theiliad left a comment

Choose a reason for hiding this comment

hlyang397 commented Sep 2, 2020

theiliad left a comment

Choose a reason for hiding this comment

theiliad commented Sep 2, 2020

hlyang397 commented Sep 3, 2020

hlyang397 commented Sep 3, 2020

theiliad left a comment

Choose a reason for hiding this comment

hlyang397 commented Sep 11, 2020

hlyang397 commented Aug 27, 2020 •

edited

Loading