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

Handle reverse support in core.scale #6343

Merged
merged 2 commits into from Jul 15, 2019
Merged

Conversation

kurkle
Copy link
Member

@kurkle kurkle commented Jun 18, 2019

  • _configure scales after placing them
  • handle ticks.reverse in getPixelForDecimal
  • add getDecimalForPixel
  • utilize getPixelForDecimal and _configure results in scales
  • some of scale.logarithmic.tests were actually targeting linear scale (this took a while to figure out)
  • -472 -403 bytes (much of this by moving helpers.log10)

Doing essentially the same thing as #6342, just a bit more abstractly.

Forked fiddle (notice horizontalBar reverse is reversed compared to 6342)

Closes #3306
Closes #6342

Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

I haven't really reviewed yet. I like the idea that we could handle reverse in core.scale. It seems like some of the scales still know about reverse though? E.g. I would have expected if all the logic is being done in core.scale that this PR would be removing reverse from scale.time

src/core/core.scale.js Outdated Show resolved Hide resolved
src/scales/scale.category.js Outdated Show resolved Hide resolved
@kurkle

This comment has been minimized.

etimberg
etimberg previously approved these changes Jun 19, 2019
src/core/core.scale.js Outdated Show resolved Hide resolved
src/core/core.scale.js Outdated Show resolved Hide resolved
src/scales/scale.category.js Outdated Show resolved Hide resolved
src/scales/scale.category.js Outdated Show resolved Hide resolved
src/scales/scale.category.js Outdated Show resolved Hide resolved
src/scales/scale.category.js Outdated Show resolved Hide resolved
@nagix
Copy link
Contributor

nagix commented Jun 19, 2019

lineValue -= scale.isHorizontal() ?
Math.max(lineValue - scale.left, scale.right - lineValue) :
Math.max(lineValue - scale.top, scale.bottom - lineValue);
These lines can be replaced with

lineValue -= Math.max(lineValue - scale._start, scale._end - lineValue);

Also,

scaleLabelX = me.left + ((me.right - me.left) / 2); // midpoint of the width
scaleLabelY = me.top + ((me.bottom - me.top) / 2);
can be:

scaleLabelX = me.left + me.width / 2; // midpoint of the width
scaleLabelY = me.top + me.height / 2;

@kurkle
Copy link
Member Author

kurkle commented Jun 19, 2019

Updated, all comments should be resolved.

src/core/core.scale.js Outdated Show resolved Hide resolved
src/core/core.scale.js Outdated Show resolved Hide resolved
src/controllers/controller.horizontalBar.js Outdated Show resolved Hide resolved
src/core/core.controller.js Show resolved Hide resolved
src/scales/scale.category.js Outdated Show resolved Hide resolved
src/scales/scale.linear.js Outdated Show resolved Hide resolved
nagix
nagix previously approved these changes Jun 24, 2019
@kurkle
Copy link
Member Author

kurkle commented Jun 24, 2019

Added _configure to scale.linearBase and scale.logarithmic. Updated scale.time to be consistent.

src/core/core.scale.js Outdated Show resolved Hide resolved
src/scales/scale.category.js Outdated Show resolved Hide resolved
src/core/core.scale.js Outdated Show resolved Hide resolved
@kurkle
Copy link
Member Author

kurkle commented Jun 25, 2019

Another update. Added offset support for linear scale to be consistent. Need to add tests for that.

image

src/core/core.scale.js Show resolved Hide resolved
src/core/core.scale.js Outdated Show resolved Hide resolved
Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

lgtm. just had one last question

test/specs/scale.category.tests.js Outdated Show resolved Hide resolved
src/scales/scale.category.js Outdated Show resolved Hide resolved
benmccann
benmccann previously approved these changes Jun 26, 2019
benmccann
benmccann previously approved these changes Jun 26, 2019
@kurkle kurkle requested review from nagix and etimberg June 27, 2019 06:25
exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
* Move log10 from core.helpers to helpers.math

* Refactor scales
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.

Reversing xAxes ticks doesn't seem to work.
4 participants