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

Add fastdom js dependency #3947

Merged
merged 1 commit into from
Dec 8, 2017
Merged

Conversation

mistercrunch
Copy link
Member

@mistercrunch mistercrunch commented Nov 28, 2017

The nvd3 docs say that it if the fastdom library is present it makes use
of it.

"Including Fastdom in your project can greatly increase the performance
of the line chart (particularly in Firefox and Internet Explorer) by
batching DOM read and write operations to avoid layout thrashing. NVD3
will take advantage of Fastdom if present."

Annectodal evidence refreshing a 4*nvd3 slices dashboard before/after fastdom
screen shot 2017-11-27 at 11 08 48 pm
screen shot 2017-11-27 at 11 13 42 pm

The nvd3 docs say that it if the fastdom library is present it makes use
of it.

"Including Fastdom in your project can greatly increase the performance
of the line chart (particularly in Firefox and Internet Explorer) by
batching DOM read and write operations to avoid layout thrashing. NVD3
will take advantage of Fastdom if present."
@graceguo-supercat graceguo-supercat merged commit 0b40c8a into apache:master Dec 8, 2017
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
The nvd3 docs say that it if the fastdom library is present it makes use
of it.

"Including Fastdom in your project can greatly increase the performance
of the line chart (particularly in Firefox and Internet Explorer) by
batching DOM read and write operations to avoid layout thrashing. NVD3
will take advantage of Fastdom if present."
@kristw
Copy link
Contributor

kristw commented Jul 28, 2018

fastdom has never been imported into the js code, so window.fastdom does not exist and nvd3 in superset did not get the benefits.

From nvd3 https://github.com/novus/nvd3/blob/master/src/dom.js

nv.dom.write = function(callback) {
	if (window.fastdom !== undefined) {
		return fastdom.mutate(callback);
	}
	return callback();
};

@mistercrunch
Copy link
Member Author

Oh makes sense, I assumed the lib could just be discovered and used as it would in Python, but that's actually not the case with JS/Webpack. So I'm guessing we have to import it explicitly.

wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
The nvd3 docs say that it if the fastdom library is present it makes use
of it.

"Including Fastdom in your project can greatly increase the performance
of the line chart (particularly in Firefox and Internet Explorer) by
batching DOM read and write operations to avoid layout thrashing. NVD3
will take advantage of Fastdom if present."
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.21.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.21.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants