Adding caret icons to max/min for column chart #5
Conversation
Jbcampbe
commented
Sep 11, 2017
•
Jbcampbe
commented
package.json
Outdated
| @@ -42,7 +42,7 @@ | |||
| "ember-cli-sri": "^2.1.0", | |||
| "ember-cli-test-loader": "^1.1.0", | |||
| "ember-cli-uglify": "^1.2.0", | |||
| "ember-data": "^2.10.0", | |||
| "ember-data": "2.14.10", | |||
There was a problem hiding this comment.
[email protected] has a regression. Pinned the version to 2.14.10 and added package-lock if we want it. If we would rather not have a lock I can remove it
There was a problem hiding this comment.
You can just pull out ember-data entirely. It's not used in this project.
|
Regarding ember-font-awesome you should be able to add it to |
| .html(() => '') | ||
| .attr('text-anchor', 'middle') | ||
| .attr('class', 'caret-icon') | ||
| .attr('x', +b.getAttribute('x') + (b.getAttribute('width') / 2)) |
There was a problem hiding this comment.
(Trivial) missing a space after the first +
There was a problem hiding this comment.
Same in the other areas where this line is present.
There was a problem hiding this comment.
When I put a space in it failed the eslint. Should I change the eslint rule?
There was a problem hiding this comment.
Oh actually I read this line wrong, I didn't realize that it's a leading +. I'm not sure why that's there, maybe to coerce it to a Number type? Probably can leave it as is then.
|
Here's an example where we lose the caret and the number itself.
|
|
oh yes @maiseyh I meant to bring that up. I asked Bradley what he wanted to happen when all values are the same. He told me in that case don't display anything. |
|
@maiseyh fixed. |
