-
Notifications
You must be signed in to change notification settings - Fork 731
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
experimental: 2x speedup of StylePanel rendering on selection change #4686
Conversation
new NanostoresSyncObject( | ||
"selectedInstanceBrowserStyle", | ||
$selectedInstanceBrowserStyle | ||
), | ||
new NanostoresSyncObject( | ||
"selectedInstanceIntanceToTag", |
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.
selectedInstanceIntanceToTag no longer synced from canvas, can be removed too
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.
Should be synced as selectedInstanceSelector can be set from canvas
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.
Ah, actually synced when selectInstance is invoked from canvas. Nevermind
// Based on this https://stackoverflow.com/questions/1248081/how-to-get-the-browser-viewport-dimensions/8876069#8876069 | ||
// this is crossbrowser way to get viewport sizes vw vh in px | ||
const vw = | ||
Math.max(document.documentElement.clientWidth, window.innerWidth) / 100; |
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.
this would probably be more accurate if it used some element as a probe, because on mobile, these vh,vw and the newer units that react to keyboard etc is hard to calculate in js
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.
not this PR issue, lets skip
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.
I would rather try to put a probe on canvas for any of this detection logic and let css/dom do it
closed in favor of this #4687 |
Description
Open this
https://p-cfc5b051-3b7a-4a59-a994-44d8d2f8b836-dot-prf.development.webstudio.is/
Start edit, and hold down button.
On my machine edit inputs are switching without serious perf issues in this PR
fast.mov
On my machine on Main https://p-cfc5b051-3b7a-4a59-a994-44d8d2f8b836-dot-main.development.webstudio.is/ it's very slow and finally hangs for a 10+ seconds
hanging.mov
Steps for reproduction
Code Review
Before requesting a review
Before merging
.env
file