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.

Sign up for GitHub

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

Refactoring to put browser specific code in a new class #3718

Merged
merged 2 commits into from
Dec 21, 2016
Merged

Refactoring to put browser specific code in a new class #3718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41f56fb
Refactoring to put browser specific code in a new class, BrowserPlatf…
etimberg Dec 17, 2016
dc59ea2
Updates per code review comments. Renamed platform.browser to platfor…
etimberg Dec 20, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/09-Advanced.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ Plugins will be called at the following times
* After datasets draw
* Resize
* Before an animation is started
* When an event occurs on the canvas (mousemove, click, etc). This requires the `options.events` property handled

Plugins should derive from Chart.PluginBase and implement the following interface
```javascript
Expand Down Expand Up @@ -437,6 +438,13 @@ Plugins should derive from Chart.PluginBase and implement the following interfac
afterDatasetsDraw: function(chartInstance, easing) { },

destroy: function(chartInstance) { }

/**
* Called when an event occurs on the chart
* @param e {Core.Event} the Chart.js wrapper around the native event. e.native is the original event
* @return {Boolean} true if the chart is changed and needs to re-render
*/
onEvent: function(chartInstance, e) {}
}
```

Expand Down
3 changes: 3 additions & 0 deletions src/chart.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ require('./core/core.legend')(Chart);
require('./core/core.interaction')(Chart);
require('./core/core.tooltip')(Chart);

// By default, we only load the browser platform.
Chart.platform = require('./platforms/platform.dom')(Chart);
Copy link
Member

Choose a reason for hiding this comment

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

Should it be Chart.Platform or Chart.platform? I thought namespaces was all lowercase and classes with a capital.

Copy link
Member Author

Choose a reason for hiding this comment

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

I used Chart.platform since it's not a constructor

Copy link
Member

Choose a reason for hiding this comment

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

Works for me, just thinking that it might have been a good idea to have some kind of naming rules for namespace and class names: I used to see namespaces in lowercase and class capitalized (whatever static or not).

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I'm fine with that. I'll change this to be Platform since it's a class


require('./elements/element.arc')(Chart);
require('./elements/element.line')(Chart);
require('./elements/element.point')(Chart);
Expand Down
156 changes: 13 additions & 143 deletions src/core/core.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,140 +14,6 @@ module.exports = function(Chart) {
// Controllers available for dataset visualization eg. bar, line, slice, etc.
Chart.controllers = {};

/**
* The "used" size is the final value of a dimension property after all calculations have
* been performed. This method uses the computed style of `element` but returns undefined
* if the computed style is not expressed in pixels. That can happen in some cases where
* `element` has a size relative to its parent and this last one is not yet displayed,
* for example because of `display: none` on a parent node.
* TODO(SB) Move this method in the upcoming core.platform class.
* @see https://developer.mozilla.org/en-US/docs/Web/CSS/used_value
* @returns {Number} Size in pixels or undefined if unknown.
*/
function readUsedSize(element, property) {
var value = helpers.getStyle(element, property);
var matches = value && value.match(/(\d+)px/);
return matches? Number(matches[1]) : undefined;
}

/**
* Initializes the canvas style and render size without modifying the canvas display size,
* since responsiveness is handled by the controller.resize() method. The config is used
* to determine the aspect ratio to apply in case no explicit height has been specified.
* TODO(SB) Move this method in the upcoming core.platform class.
*/
function initCanvas(canvas, config) {
var style = canvas.style;

// NOTE(SB) canvas.getAttribute('width') !== canvas.width: in the first case it
// returns null or '' if no explicit value has been set to the canvas attribute.
var renderHeight = canvas.getAttribute('height');
var renderWidth = canvas.getAttribute('width');

// Chart.js modifies some canvas values that we want to restore on destroy
canvas._chartjs = {
initial: {
height: renderHeight,
width: renderWidth,
style: {
display: style.display,
height: style.height,
width: style.width
}
}
};

// Force canvas to display as block to avoid extra space caused by inline
// elements, which would interfere with the responsive resize process.
// https://github.com/chartjs/Chart.js/issues/2538
style.display = style.display || 'block';

if (renderWidth === null || renderWidth === '') {
var displayWidth = readUsedSize(canvas, 'width');
if (displayWidth !== undefined) {
canvas.width = displayWidth;
}
}

if (renderHeight === null || renderHeight === '') {
if (canvas.style.height === '') {
// If no explicit render height and style height, let's apply the aspect ratio,
// which one can be specified by the user but also by charts as default option
// (i.e. options.aspectRatio). If not specified, use canvas aspect ratio of 2.
canvas.height = canvas.width / (config.options.aspectRatio || 2);
} else {
var displayHeight = readUsedSize(canvas, 'height');
if (displayWidth !== undefined) {
canvas.height = displayHeight;
}
}
}

return canvas;
}

/**
* Restores the canvas initial state, such as render/display sizes and style.
* TODO(SB) Move this method in the upcoming core.platform class.
*/
function releaseCanvas(canvas) {
if (!canvas._chartjs) {
return;
}

var initial = canvas._chartjs.initial;
['height', 'width'].forEach(function(prop) {
var value = initial[prop];
if (value === undefined || value === null) {
canvas.removeAttribute(prop);
} else {
canvas.setAttribute(prop, value);
}
});

helpers.each(initial.style || {}, function(value, key) {
canvas.style[key] = value;
});

// The canvas render size might have been changed (and thus the state stack discarded),
// we can't use save() and restore() to restore the initial state. So make sure that at
// least the canvas context is reset to the default state by setting the canvas width.
// https://www.w3.org/TR/2011/WD-html5-20110525/the-canvas-element.html
canvas.width = canvas.width;

delete canvas._chartjs;
}

/**
* TODO(SB) Move this method in the upcoming core.platform class.
*/
function acquireContext(item, config) {
if (typeof item === 'string') {
item = document.getElementById(item);
} else if (item.length) {
// Support for array based queries (such as jQuery)
item = item[0];
}

if (item && item.canvas) {
// Support for any object associated to a canvas (including a context2d)
item = item.canvas;
}

if (item instanceof HTMLCanvasElement) {
// To prevent canvas fingerprinting, some add-ons undefine the getContext
// method, for example: https://github.com/kkapsner/CanvasBlocker
// https://github.com/chartjs/Chart.js/issues/2807
var context = item.getContext && item.getContext('2d');
if (context instanceof CanvasRenderingContext2D) {
initCanvas(item, config);
return context;
}
}

return null;
}

/**
* Initializes the given config with global and chart default values.
*/
Expand Down Expand Up @@ -197,7 +63,7 @@ module.exports = function(Chart) {

config = initConfig(config);

var context = acquireContext(item, config);
var context = Chart.platform.acquireContext(item, config);
var canvas = context && context.canvas;
var height = canvas && canvas.height;
var width = canvas && canvas.width;
Expand Down Expand Up @@ -696,7 +562,7 @@ module.exports = function(Chart) {
helpers.unbindEvents(me, me.events);
helpers.removeResizeListener(canvas.parentNode);
helpers.clear(me.chart);
releaseCanvas(canvas);
Chart.platform.releaseContext(me.chart.ctx);
me.chart.canvas = null;
me.chart.ctx = null;
}
Expand Down Expand Up @@ -742,17 +608,19 @@ module.exports = function(Chart) {

eventHandler: function(e) {
var me = this;
var legend = me.legend;
var tooltip = me.tooltip;
var hoverOptions = me.options.hover;

// Buffer any update calls so that renders do not occur
me._bufferedRender = true;
me._bufferedRequest = null;

var changed = me.handleEvent(e);
changed |= legend && legend.handleEvent(e);
changed |= tooltip && tooltip.handleEvent(e);
// Create platform agnostic chart event using platform specific code
var chartEvent = Chart.platform.createEvent(e, me.chart);

var changed = me.handleEvent(chartEvent);
changed |= tooltip && tooltip.handleEvent(chartEvent);
changed |= Chart.plugins.notify(me, 'onEvent', [chartEvent]);

var bufferedRequest = me._bufferedRequest;
if (bufferedRequest) {
Expand All @@ -776,7 +644,7 @@ module.exports = function(Chart) {
/**
* Handle an event
* @private
* param e {Event} the event to handle
* param e {Core.Event} the event to handle
* @return {Boolean} true if the chart needs to re-render
*/
handleEvent: function(e) {
Expand All @@ -796,12 +664,14 @@ module.exports = function(Chart) {

// On Hover hook
if (hoverOptions.onHover) {
hoverOptions.onHover.call(me, e, me.active);
// Need to call with native event here to not break backwards compatibility
hoverOptions.onHover.call(me, e.native, me.active);
}

if (e.type === 'mouseup' || e.type === 'click') {
if (options.onClick) {
options.onClick.call(me, e, me.active);
// Use e.native here for backwards compatibility
options.onClick.call(me, e.native, me.active);
}
}

Expand Down
31 changes: 24 additions & 7 deletions src/core/core.interaction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,23 @@
module.exports = function(Chart) {
var helpers = Chart.helpers;

/**
* Helper function to get relative position for an event
* @param e {Event|Core.Event} the event to get the position for
* @param chart {chart} the chart
* @returns {Point} the event position
*/
function getRelativePosition(e, chart) {
if (e.native) {
return {
x: e.x,
y: e.y
};
}

return helpers.getRelativePosition(e, chart);
}

/**
* Helper function to traverse all of the visible elements in the chart
* @param chart {chart} the chart
Expand Down Expand Up @@ -82,7 +99,7 @@ module.exports = function(Chart) {
}

function indexMode(chart, e, options) {
var position = helpers.getRelativePosition(e, chart.chart);
var position = getRelativePosition(e, chart.chart);
var distanceMetric = function(pt1, pt2) {
return Math.abs(pt1.x - pt2.x);
};
Expand Down Expand Up @@ -125,7 +142,7 @@ module.exports = function(Chart) {
// Helper function for different modes
modes: {
single: function(chart, e) {
var position = helpers.getRelativePosition(e, chart.chart);
var position = getRelativePosition(e, chart.chart);
var elements = [];

parseVisibleItems(chart, function(element) {
Expand Down Expand Up @@ -166,7 +183,7 @@ module.exports = function(Chart) {
* @return {Chart.Element[]} Array of elements that are under the point. If none are found, an empty array is returned
*/
dataset: function(chart, e, options) {
var position = helpers.getRelativePosition(e, chart.chart);
var position = getRelativePosition(e, chart.chart);
var items = options.intersect ? getIntersectItems(chart, position) : getNearestItems(chart, position, false);

if (items.length > 0) {
Expand All @@ -193,7 +210,7 @@ module.exports = function(Chart) {
* @return {Chart.Element[]} Array of elements that are under the point. If none are found, an empty array is returned
*/
point: function(chart, e) {
var position = helpers.getRelativePosition(e, chart.chart);
var position = getRelativePosition(e, chart.chart);
return getIntersectItems(chart, position);
},

Expand All @@ -206,7 +223,7 @@ module.exports = function(Chart) {
* @return {Chart.Element[]} Array of elements that are under the point. If none are found, an empty array is returned
*/
nearest: function(chart, e, options) {
var position = helpers.getRelativePosition(e, chart.chart);
var position = getRelativePosition(e, chart.chart);
var nearestItems = getNearestItems(chart, position, options.intersect);

// We have multiple items at the same distance from the event. Now sort by smallest
Expand Down Expand Up @@ -238,7 +255,7 @@ module.exports = function(Chart) {
* @return {Chart.Element[]} Array of elements that are under the point. If none are found, an empty array is returned
*/
x: function(chart, e, options) {
var position = helpers.getRelativePosition(e, chart.chart);
var position = getRelativePosition(e, chart.chart);
var items = [];
var intersectsItem = false;

Expand Down Expand Up @@ -269,7 +286,7 @@ module.exports = function(Chart) {
* @return {Chart.Element[]} Array of elements that are under the point. If none are found, an empty array is returned
*/
y: function(chart, e, options) {
var position = helpers.getRelativePosition(e, chart.chart);
var position = getRelativePosition(e, chart.chart);
var items = [];
var intersectsItem = false;

Expand Down
20 changes: 14 additions & 6 deletions src/core/core.legend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ module.exports = function(Chart) {
/**
* Handle an event
* @private
* @param e {Event} the event to handle
* @param e {Core.Event} the event to handle
* @return {Boolean} true if a change occured
*/
handleEvent: function(e) {
Expand All @@ -460,9 +460,9 @@ module.exports = function(Chart) {
return;
}

var position = helpers.getRelativePosition(e, me.chart.chart),
x = position.x,
y = position.y;
// Chart event already has relative position in it
var x = e.x,
y = e.y;

if (x >= me.left && x <= me.right && y >= me.top && y <= me.bottom) {
// See if we are touching one of the dataset boxes
Expand All @@ -473,11 +473,13 @@ module.exports = function(Chart) {
if (x >= hitBox.left && x <= hitBox.left + hitBox.width && y >= hitBox.top && y <= hitBox.top + hitBox.height) {
// Touching an element
if (type === 'click') {
opts.onClick.call(me, e, me.legendItems[i]);
// use e.native for backwards compatibility
opts.onClick.call(me, e.native, me.legendItems[i]);
changed = true;
break;
} else if (type === 'mousemove') {
opts.onHover.call(me, e, me.legendItems[i]);
// use e.native for backwards compatibility
opts.onHover.call(me, e.native, me.legendItems[i]);
changed = true;
break;
}
Expand Down Expand Up @@ -523,6 +525,12 @@ module.exports = function(Chart) {
Chart.layoutService.removeBox(chartInstance, chartInstance.legend);
delete chartInstance.legend;
}
},
onEvent: function(chartInstance, e) {
var legend = chartInstance.legend;
if (legend) {
legend.handleEvent(e);
}
}
});
};
7 changes: 5 additions & 2 deletions src/core/core.tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ module.exports = function(Chart) {
/**
* Handle an event
* @private
* @param e {Event} the event to handle
* @param e {Core.Event} the event to handle
* @returns {Boolean} true if the tooltip changed
*/
handleEvent: function(e) {
Expand All @@ -785,7 +785,10 @@ module.exports = function(Chart) {
me._lastActive = me._active;

if (options.enabled || options.custom) {
me._eventPosition = helpers.getRelativePosition(e, me._chart);
me._eventPosition = {
x: e.x,
y: e.y
};

var model = me._model;
me.update(true);
Expand Down
Loading