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

Desktop: Performance: fixes false dependencies between MainScreen and UserWebview #6446

Closed
wants to merge 1 commit into from

Conversation

ken1kob
Copy link
Contributor

@ken1kob ken1kob commented Apr 23, 2022

This PR is one of PRs resolving issue #6386 and improves various UI responses.

MainScreenComponent has pluginHtmlContents property in its props, but it is needed not by MainScreenComponent itself but by UserWebview and UserWebviewDialog. This false dependency causes redundant re-rendering. Because when pluginHtmlContents property changes, MainScreenComponent is re-rendered, and then its children components (almost all except MenuBar) are also re-rendered.

Treatments in this PR:

  • pluginHtmlContents is removed from MainScreenComponent.
  • A dependency between MainScreenComponents and UserWebview/UserWebviewDialog, i.e. html property are removed in MainScreenComponent.
  • html property is directly referred by UserWebview through mapStateToProps.

@tessus tessus added the desktop All desktop platforms label Aug 14, 2022
@laurent22
Copy link
Owner

@ken1kob, I will close these pull requests because even if we do optimise these parts of the code we will follow a different approach, which will probably start by some refactoring. Also I believe some of these performance issue no longer apply due to recent changes.

I appreciate anyway you taking the time to look into this, and apologies for not merging these particular PRs.

@laurent22 laurent22 closed this Nov 19, 2023
Repository owner locked as resolved and limited conversation to collaborators Nov 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
desktop All desktop platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants