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

iss3917 #3958

Closed
wants to merge 43 commits into from
Closed

iss3917 #3958

wants to merge 43 commits into from

Conversation

roicos
Copy link
Contributor

@roicos roicos commented Feb 25, 2017

Dear colleges!

Please, consider the pull request for iss#3917

This is how the chart looks after fix:
iss3917-fix

@jakesylvestre
Copy link
Contributor

@roicos Can you resolve the merge conflict/rebase?

@etimberg etimberg added this to the Version 2.7 milestone Apr 22, 2017
if (newVal === 0) {
pixel = zero;
} else if (newVal === min) {
pixel = tickOpts.reverse ? zero + innerDimension * 0.04 : zero - innerDimension * 0.04;
Copy link
Member

Choose a reason for hiding this comment

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

My concern here is that the heuristic of 4% ( * 0.04) isn't guaranteed to work for all charts, particularly small ones. What may need to happen instead is that innerDimension is reduced by fontSize + spacing to always make room for the 0 tick.

The axis would need to store where exactly 0 is so that when getPixelForValue(0) is run, we return the right thing.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -198,29 +199,23 @@ module.exports = function(Chart) {
} else {
// Bottom - top since pixels increase downward on a screen
innerDimension = me.height;
Copy link
Member

Choose a reason for hiding this comment

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

We should also support 0 for log axes that are horizontal, not just vertical

@benmccann
Copy link
Contributor

@roicos would you be able to rebase this PR and address @etimberg's comments?

roicos and others added 20 commits August 18, 2017 14:44
If a value is set on the model after `pivot()` has been called, the view wasn't initialized and the animation started from 0. Now, `_start` and incomplete `_view` are initialized to the model value during the transition (no initial implicit transition).

Also remove exception handling when animating a string (color), which is faster when string are not valid colors (e.g. tooltip position). It requires to update `chartjs-color` to version 2.1.0.
Animation callbacks now receives `animationObject` directly with a reference on the associated chart (`animation.chart`), which deprecates `animation.animationObject` and `animation.chartInstance`. Also fix missing `onComplete` animation argument and make sure that an animation object is passed even when animations are disabled.
… new settings.

gridLines.circular is a new option that toggles circular lines. This allows radar charts with circular lines chartjs#3082
pointLabels.display is a new option that toggles the display of point labels.
The existing angleLines.display is used with the new pointLabels.display setting is used to trigger the radar like settings.
This required changing the default polar area config.
Added a `maxBarThickness` setting for bar charts xAxis
Prevent attempt to remove the legend or title layout items if they haven't been created but also check if the item to remove is registered with the layout manager to avoid removing the wrong box `splice(-1, 1)`. Add ids to the legend and title plugins to allow to fully disable them (`options: {plugins: {legend: false, title: false}}`).
`karma.conf.ci.js` has been merged into `karma.conf.js` for local testing consistency: `gulp unittestWatch` has been replaced by `gulp unittest --watch` and thus use exactly the same config file. Upgrade to latest jasmine and karma packages and remove deprecated `gulp-karma` dependency (directly use `karma.Server` in gulp).

Split `test/mockContext.js` into smaller `test/jasmine.*` modules to make easier unit tests maintenance and finally, move all `*.test.js` files under the `test/specs` folder.
Attempt to make easier the creation of unit tests that check the drawing output. Until now, this was done by checking calls on a 'fake' context, which is hard to maintain (need to update pixel values by hands) and also not reliable when optimizing code (i.e. different calls sequence but same result).

As of now, it's possible to define 'auto' tests based on JSON/PNG fixtures: chart is generated from the JSON file and compared to the associated PNG image. The image diff is done using `pixelmatch`. As an example (and in preparation of the `filler` plugin), add auto tests for the line element `fill` options.
The `fill` option now accepts the index of the target dataset (number) or a string starting by "+" or "-" followed by a number representing the dataset index relative to the current one (e.g. `fill: "-2"` on dataset at index 3 will fill to dataset at index 1). It's also possible to "propagate" the filling to the target of an hidden dataset (`options.plugins.filler.propagate`). Fill boundaries `zero`, `top` and `bottom` have been deprecated and replaced by `origin`, `start` and `end`.

Implementation has been moved out of the line element into a new plugin (`src/plugins/plugin.filler.js`) and does not rely anymore on the deprecated model `scaleTop`, `scaleBottom` and `scaleZero` values. Drawing Bézier splines has been refactored in the canvas helpers (note that `Chart.helpers.canvas` is now an alias of `Chart.canvasHelpers`).

Add 3 new examples and extend utils with a pseudo-random number generator that can be initialized with `srand`. That makes possible to design examples starting always with the same initial data.
Fix the `readUsedSize` regular expression to correctly parse (truncate) pixel decimal values.
Update the docs structure/content to use GitBook
etimberg and others added 21 commits August 20, 2017 22:02
* Undo fix for chartjs#3585 since it has broken the stacked bar charts when the axis has a user defined minimum value.

* When a value of 0 is requested for a vertical logarithmic axis, return the bottom point
Radar chart position is not center horizontally with v2.5.0.

Right and left of `furthestLimits` would be switched wrongly on
this refactoring commit.
chartjs@e1606f8
* Add of zero line border dash options
* Update Readme with zero line border dash config options
Add window scroll position to tooltip position calculation so they show up in the intended place instead of potentially off-screen! Moved tooltips inside the canvas parent container since they are being positioned in terms of its location
Coverage data are now generated by running `gulp unittest` with the `--coverage` argument: unit tests are then executed a single time on Travis. The gulp `coverage` task has been removed and `karma.coverage.conf.ci.js` merged into `karma.conf.ci.js`.

Update documentation with gulp commands (and remove them from `README.md`) and remove unused `config.jshintrc` (oversight from chartjs#3256). Delete `thankyou.md` which has been merged into `README.md`.
* Fixed issue, that category scale shows data points misplaced. See chartjs#4060
* Cleaned code
* Fixed Irregular whitespace
* Fixed error in case value is undefined
* Verified that no error is thrown in case value === null
… value was essentially hard coded to

 2 because of a typo that read it from the positioner output. Based on chartjs#3599 we agreed to make this into
a config setting.
* Make parseTime private
* start on fixing time scale
* Reimplement existing functionality
* Tidy tests
* Fix labels for non-linearly sized units

Months, quarters and years have non-constant numbers of seconds. A scale that's linear WRT milliseconds produces incorrect tick labels due to the label formatting losing precision (eg year labels lose month and day so a label of 2016-12-32 displays as 2016 instead of 2017).

* Re-implement tick generation

As in v2.5
…sly the user got a message about `type` being undefined.

This gives something that's easier to understand and debug.
…artjs#4094)

* respect new scale option 'order' when ordering scales

* scale service - respect new weight scale option for axes ordering

* added test for scale ordering by weight

* removed trailing spaces from layout weight scale order test
@roicos roicos closed this Aug 23, 2017
@roicos roicos deleted the pr20022017 branch August 23, 2017 04:49
@simonbrunel simonbrunel removed this from the Version 2.7 milestone Aug 28, 2017
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.

None yet