Skip to content

De-jankify window resizing#1652

Merged
gettinToasty merged 7 commits intostagingfrom
sb_window_dejank
Apr 30, 2019
Merged

De-jankify window resizing#1652
gettinToasty merged 7 commits intostagingfrom
sb_window_dejank

Conversation

@gettinToasty
Copy link
Copy Markdown
Contributor

This PR does a couple things--

  1. Makes out Electron windows transparent which removes the white flash we see on resizes
  2. Add more disabling of style blocking elements, specifically to all window resizes and other places we use animations

Slight side effect that when resizing a child window the preview on the main window will also be blocked from displaying which is a little weird but overall a much smoother experience. Might be worth isolating the style-blocking elements on a per-window basis in a future iteration

@avacreeth
Copy link
Copy Markdown
Member

Ty for taking the initiative on this. After playing around with the branch I think this is a definite improvement in jank reduction. I'm wondering however if we should try to tackle the 1 remaining piece of jank that you mentioned. Theoretically we should be able to make hideStyleBlockingElements to a different service (it doesn't really belong in customization anymore anyway) and then add some mechanism in to prevent this mutation from getting synced to other windows (I can help with this). I think that should result in the behavior you want.

@avacreeth
Copy link
Copy Markdown
Member

I think we should have QA play with this PR. This touches a lot of high risk areas. Especially the ChildWindow component which is really really brittle and does some very complex things.

Comment thread app/components/windows/Main.vue.ts Outdated
Comment thread app/components/windows/Main.vue.ts Outdated
hideStyleBlockers: options.hideStyleBlockers,
});
this.setWindowTitle();
window.addEventListener('resize', this.windowSizeHandler);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems a little complex to be adding and removing the resize listener every time we mount and unmount a component to the window. Is the a reason we can't just have the the resize handler mount when the ChildWindow component mounts and just keep it there?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

using the vue lifecycle methods was giving me some extremely weird bugs when i was testing that out, so this was the closest compromise i could find

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm. Could be related to the fact that we are hiding the window without destroying it.

Comment thread app/components/windows/ChildWindow.vue.ts Outdated
Comment thread app/components/windows/OneOffWindow.vue.ts Outdated
@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit be24c5c and detected 0 issues on this pull request.

View more on Code Climate.

@gettinToasty gettinToasty merged commit 73b7502 into staging Apr 30, 2019
@gettinToasty gettinToasty deleted the sb_window_dejank branch April 30, 2019 18:19
@summeroff summeroff mentioned this pull request Apr 9, 2026
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.

2 participants