-
Notifications
You must be signed in to change notification settings - Fork 8.5k
move YAxis inside Axis #8253
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
move YAxis inside Axis #8253
Changes from all commits
ff2aa55
ec934e5
8053ea1
cb77002
0b3506b
39cc117
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ import $ from 'jquery'; | |
| import _ from 'lodash'; | ||
| import moment from 'moment'; | ||
| import VislibLibErrorHandlerProvider from 'ui/vislib/lib/_error_handler'; | ||
| import errors from 'ui/errors'; | ||
|
|
||
| export default function AxisFactory(Private) { | ||
|
|
||
| const ErrorHandler = Private(VislibLibErrorHandlerProvider); | ||
|
|
@@ -21,9 +23,13 @@ export default function AxisFactory(Private) { | |
| this.el = args.el; | ||
| this.xValues = args.xValues; | ||
| this.ordered = args.ordered; | ||
| this.xAxisFormatter = args.xAxisFormatter; | ||
| this.axisFormatter = args.type === 'category' ? args.xAxisFormatter : args.yAxisFormatter; | ||
| this.expandLastBucket = args.expandLastBucket == null ? true : args.expandLastBucket; | ||
| this._attr = _.defaults(args._attr || {}); | ||
| this.scale = null; | ||
| this.domain = [args.yMin, args.yMax]; | ||
| this.elSelector = args.type === 'category' ? '.x-axis-div' : '.y-axis-div'; | ||
| this.type = args.type; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can move this up, and use |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -33,7 +39,7 @@ export default function AxisFactory(Private) { | |
| * @returns {D3.UpdateSelection} Appends x axis to visualization | ||
| */ | ||
| render() { | ||
| d3.select(this.el).selectAll('.x-axis-div').call(this.draw()); | ||
| d3.select(this.el).selectAll(this.elSelector).call(this.draw()); | ||
| }; | ||
|
|
||
| /** | ||
|
|
@@ -221,7 +227,7 @@ export default function AxisFactory(Private) { | |
| this.xAxis = d3.svg.axis() | ||
| .scale(this.xScale) | ||
| .ticks(10) | ||
| .tickFormat(this.xAxisFormatter) | ||
| .tickFormat(this.axisFormatter) | ||
| .orient('bottom'); | ||
| }; | ||
|
|
||
|
|
@@ -234,6 +240,9 @@ export default function AxisFactory(Private) { | |
| draw() { | ||
| const self = this; | ||
| this._attr.isRotated = false; | ||
| const margin = this._attr.margin; | ||
| const mode = this._attr.mode; | ||
| const isWiggleOrSilhouette = (mode === 'wiggle' || mode === 'silhouette'); | ||
|
|
||
| return function (selection) { | ||
| const n = selection[0].length; | ||
|
|
@@ -247,21 +256,48 @@ export default function AxisFactory(Private) { | |
| const width = parentWidth / n; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||
| const height = $(this.parentElement).height(); | ||
|
|
||
| self.validateWidthandHeight(width, height); | ||
| /* | ||
| const width = $(el).parent().width(); | ||
| const height = $(el).height(); | ||
| */ | ||
| const adjustedHeight = height - margin.top - margin.bottom; | ||
|
|
||
| self.getXAxis(width); | ||
|
|
||
| self.validateWidthandHeight(width, height); | ||
|
|
||
| const svg = div.append('svg') | ||
| .attr('width', width) | ||
| .attr('height', height); | ||
|
|
||
| svg.append('g') | ||
| .attr('class', 'x axis') | ||
| .attr('transform', 'translate(0,0)') | ||
| .call(self.xAxis); | ||
| if (self.type === 'category') { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably just move these two branches of code into helper functions,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as in the following PRs i would make both axis to be more alike this IF statement will go away. |
||
| self.getXAxis(width); | ||
| svg.append('g') | ||
| .attr('class', 'x axis') | ||
| .attr('transform', 'translate(0,0)') | ||
| .call(self.xAxis); | ||
| } else { | ||
| const yAxis = self.getYAxis(adjustedHeight); | ||
| if (!isWiggleOrSilhouette) { | ||
| svg.append('g') | ||
| .attr('class', 'y axis') | ||
| .attr('transform', 'translate(' + (width - 2) + ',' + margin.top + ')') | ||
| .call(yAxis); | ||
|
|
||
| const container = svg.select('g.y.axis').node(); | ||
| if (container) { | ||
| const cWidth = Math.max(width, container.getBBox().width); | ||
| svg.attr('width', cWidth); | ||
| svg.select('g') | ||
| .attr('transform', 'translate(' + (cWidth - 2) + ',' + margin.top + ')'); | ||
| } | ||
| } | ||
|
|
||
| } | ||
| }); | ||
|
|
||
| selection.call(self.filterOrRotate()); | ||
| if (self.type === 'category') { | ||
| selection.call(self.filterOrRotate()); | ||
| } | ||
| }; | ||
| }; | ||
|
|
||
|
|
@@ -346,6 +382,153 @@ export default function AxisFactory(Private) { | |
| }; | ||
| }; | ||
|
|
||
| _isPercentage() { | ||
| return (this._attr.mode === 'percentage'); | ||
| }; | ||
|
|
||
| _isUserDefined() { | ||
| return (this._attr.setYExtents); | ||
| }; | ||
|
|
||
| _isYExtents() { | ||
| return (this._attr.defaultYExtents); | ||
| }; | ||
|
|
||
| _validateUserExtents(domain) { | ||
| const self = this; | ||
|
|
||
| return domain.map(function (val) { | ||
| val = parseInt(val, 10); | ||
|
|
||
| if (isNaN(val)) throw new Error(val + ' is not a valid number'); | ||
| if (self._isPercentage() && self._attr.setYExtents) return val / 100; | ||
| return val; | ||
| }); | ||
| }; | ||
|
|
||
| _getExtents(domain) { | ||
| const min = domain[0]; | ||
| const max = domain[1]; | ||
|
|
||
| if (this._isUserDefined()) return this._validateUserExtents(domain); | ||
| if (this._isYExtents()) return domain; | ||
| if (this._attr.scale === 'log') return this._logDomain(min, max); // Negative values cannot be displayed with a log scale. | ||
| if (!this._isYExtents() && !this._isUserDefined()) return [Math.min(0, min), Math.max(0, max)]; | ||
| return domain; | ||
| }; | ||
|
|
||
| _throwCustomError(message) { | ||
| throw new Error(message); | ||
| }; | ||
|
|
||
| _throwLogScaleValuesError() { | ||
| throw new errors.InvalidLogScaleValues(); | ||
| }; | ||
|
|
||
| /** | ||
| * Returns the appropriate D3 scale | ||
| * | ||
| * @param fnName {String} D3 scale | ||
| * @returns {*} | ||
| */ | ||
| _getScaleType(fnName) { | ||
| if (fnName === 'square root') fnName = 'sqrt'; // Rename 'square root' to 'sqrt' | ||
| fnName = fnName || 'linear'; | ||
|
|
||
| if (typeof d3.scale[fnName] !== 'function') return this._throwCustomError('YAxis.getScaleType: ' + fnName + ' is not a function'); | ||
|
|
||
| return d3.scale[fnName](); | ||
| }; | ||
|
|
||
| /** | ||
| * Return the domain for log scale, i.e. the extent of the log scale. | ||
| * Log scales must begin at 1 since the log(0) = -Infinity | ||
| * | ||
| * @param {Number} min | ||
| * @param {Number} max | ||
| * @returns {Array} | ||
| */ | ||
| _logDomain(min, max) { | ||
| if (min < 0 || max < 0) return this._throwLogScaleValuesError(); | ||
| return [1, max]; | ||
| }; | ||
|
|
||
| /** | ||
| * Creates the d3 y scale function | ||
| * | ||
| * @method getYScale | ||
| * @param height {Number} DOM Element height | ||
| * @returns {D3.Scale.QuantitiveScale|*} D3 yScale function | ||
| */ | ||
| getYScale(height) { | ||
| const scale = this._getScaleType(this._attr.scale); | ||
| const domain = this._getExtents(this.domain); | ||
|
|
||
| this.yScale = scale | ||
| .domain(domain) | ||
| .range([height, 0]); | ||
|
|
||
| if (!this._isUserDefined()) this.yScale.nice(); // round extents when not user defined | ||
| // Prevents bars from going off the chart when the y extents are within the domain range | ||
| if (this._attr.type === 'histogram') this.yScale.clamp(true); | ||
| return this.yScale; | ||
| }; | ||
|
|
||
| getScaleType() { | ||
| return this._attr.scale; | ||
| }; | ||
|
|
||
| tickFormat() { | ||
| const isPercentage = this._attr.mode === 'percentage'; | ||
| if (isPercentage) return d3.format('%'); | ||
| if (this.axisFormatter) return this.axisFormatter; | ||
| return d3.format('n'); | ||
| }; | ||
|
|
||
| _validateYScale(yScale) { | ||
| if (!yScale || _.isNaN(yScale)) throw new Error('yScale is ' + yScale); | ||
| }; | ||
|
|
||
| /** | ||
| * Creates the d3 y axis function | ||
| * | ||
| * @method getYAxis | ||
| * @param height {Number} DOM Element height | ||
| * @returns {D3.Svg.Axis|*} D3 yAxis function | ||
| */ | ||
| getYAxis(height) { | ||
| const yScale = this.getYScale(height); | ||
| this._validateYScale(yScale); | ||
|
|
||
| // Create the d3 yAxis function | ||
| this.yAxis = d3.svg.axis() | ||
| .scale(yScale) | ||
| .tickFormat(this.tickFormat(this.domain)) | ||
| .ticks(this.tickScale(height)) | ||
| .orient('left'); | ||
|
|
||
| return this.yAxis; | ||
| }; | ||
|
|
||
| /** | ||
| * Create a tick scale for the y axis that modifies the number of ticks | ||
| * based on the height of the wrapping DOM element | ||
| * Avoid using even numbers in the yTickScale.range | ||
| * Causes the top most tickValue in the chart to be missing | ||
| * | ||
| * @method tickScale | ||
| * @param height {Number} DOM element height | ||
| * @returns {number} Number of y axis ticks | ||
| */ | ||
| tickScale(height) { | ||
| const yTickScale = d3.scale.linear() | ||
| .clamp(true) | ||
| .domain([20, 40, 1000]) | ||
| .range([0, 3, 11]); | ||
|
|
||
| return Math.ceil(yTickScale(height)); | ||
| }; | ||
|
|
||
| /** | ||
| * Returns a string that is truncated to fit size | ||
| * | ||
|
|
@@ -404,7 +587,7 @@ export default function AxisFactory(Private) { | |
|
|
||
| if ((startX + halfWidth) < myX && maxW > (myX + halfWidth)) { | ||
| startX = myX + halfWidth; | ||
| return self.xAxisFormatter(d); | ||
| return self.axisFormatter(d); | ||
| } else { | ||
| d3.select(this.parentNode).remove(); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is domain only used by the yAxis? It would make sense for xAxis too right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its only used for yAxis ... the code in this PR is pretty bad as i only joined the 2 files to make it clear how i progressed. yAxis and xAxis have many functions that actually do same/similar thing but are named differently and sometimes work in different ways (like on one axis you are supposed to pass a scale to function which returns domain and on the other one you are supposed to pass in limits and then apply the return value on scale ... i wouldn't worry about all that in this PR ... its just to show steps of progress ... so putting all the functions from both files into one ... then we can clearly see in the diffs what have been changed from here on ...