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

Update satus.js correct Menu load timing problems #2160 #2163

Merged
merged 10 commits into from
May 6, 2024

Conversation

raszpl
Copy link
Contributor

@raszpl raszpl commented Apr 4, 2024

should solve #2160

@ImprovedTube
Copy link
Member

ImprovedTube commented Apr 4, 2024

Good day! :) Without loading the JS in the head, the later ones frequently aren't loaded or executed. (put CSS in the end on purpose too)


( We can remove the "overlay" in satus.js line 910 after callback(items); )

@ImprovedTube ImprovedTube self-assigned this Apr 4, 2024
@raszpl
Copy link
Contributor Author

raszpl commented Apr 4, 2024

Without loading the JS in the head, the later ones frequently aren't loaded.

What do you mean? Can you reproduce a case of js not loading?

(put CSS in the end on purpose too)

the sooner CSS is loaded the better, loading CSS late results in flashing colors if browser applies styles after showing html early.

( We can remove the "overlay" in satus.js line 910 after callback(items); )

the "problem" (if you can call additional 200ms that) is manipulating dom by adding overlay in the first place. There was never any need for removing it after menu loaded, it can stay there under the menu, its invisible anyway.

@ImprovedTube
Copy link
Member

#1803 maybe you saw the bug 1 or 2 years ago with not all of the red front screen buttons appearing.

BTW just to make sure, hope you like the threads. Sometimes i might just add vague notes, but then i can accept PRs and change back something.

the "problem" (if you can call additional 200ms that) is manipulating dom by adding overlay in the first place.

200ms is shocking.

There was never any need for removing it after menu loaded, it can stay there under the menu, its invisible anyway.

👍 Removing anyways just felt clean/futureproof, since we are at it anyways. And might potentially stop the animation before it even started. Satus.js & ImprovedTube was(/will be) template for more extensions like https://chromewebstore.google.com/detail/unlock-keyboard-mouse/ijngdimmjkngoglcjaheoadciaalbafl, where we typically load fewer settings.
More relevant (also for trouble shooting #2160) literally "...asking your browser what settings you made here before..." still expires as soon as settings are loaded. (=Could alternatively be edited into "...we don't know why the menu didn't appear yet..." )

@raszpl
Copy link
Contributor Author

raszpl commented Apr 6, 2024

200ms is shocking.

it is! thats why I described how to masure so you can verify for yourself

#2160 (comment)
modify:

satus.storage.import(function (items) {
like this:

console.time('doSomething')
satus.storage.import(function (items) {
	var language = items.language;
    if (!language || language === 'default') {	language = false;}
	satus.locale.import(language, function () {
		satus.render(extension.skeleton);

		extension.exportSettings();
		extension.importSettings();

		satus.parentify(extension.skeleton, [
			'attr',
			'baseProvider',
			'layersProvider',
			'rendered',
			'storage',
			'parentObject',
			'parentSkeleton',
			'childrenContainer',
			'value'
		]);

		extension.attributes();
		satus.events.on('storage-set', extension.attributes);
	}, '_locales/');
});
console.timeEnd('doSomething')

open extension popup, open devtools inside the popup and check the number in console before and after commenting out 5 lines (899-902, 910) patched out of satus.js in this patch. This measures total time taken by settings and locales being loaded. Its possible this only speeds up popup in Vivaldi for example (I dont use/test under ff/pure chrome), things like this should always be measured.

There was never any need for removing it after menu loaded, it can stay there under the menu, its invisible anyway.

👍 Removing anyways just felt clean/futureproof

to remove it js has to run after id="overlay" is created. Browser executes js as soon as its loaded, thats why in the patch js files were moved below id="overlay". Edit: but this overlay is not needed in the first place after fixing timing issues.

@raszpl raszpl changed the title Update satus.js cut ~200ms from popup creation time Update satus.js correct Menu load timing problems #2160 Apr 6, 2024
@ImprovedTube ImprovedTube merged commit 75ee439 into code-charity:master May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants