-
-
Notifications
You must be signed in to change notification settings - Fork 481
[Tech] UI performance improvements + small fixes #2010
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
Conversation
…uncher into other/render_optimizations
@@ -1,2 +1,2 @@ | |||
#!/bin/bash | |||
yarn codecheck && yarn lint && yarn prettier-fix && yarn i18n |
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.
Also removing this command from pre-push because on Windows, it overwrites all file permissions and we need to be stashing the changes, which is problematic.
I also think this is not pretty useful at this stage, but I have no strong opinion about it.
I'm always a bit worried when I see memo cause I'm never sure what will trigger the re-renders so can't review it in that sense. But I played around with the PR and it does feel faster and I tried clicking things, install/uninstall, filtering, etc, all seems to be working fine. |
I agree 100%, memo, useMemo and useCallback should not be used always, but I think here makes sense because it is a static app. But it is good to test if all states that needs update are being updated. Since we have the context the control most things, I think they are working fine. But my feeling is that Heroic is a bit faster to navigate and also the removal of the getGameInfo on gamepage makes it loads instantly now, that's good as well. |
…Launcher/HeroicGamesLauncher into other/render_optimizations
This one adds some performance improvements mostly on Library and GamePage.
I am doing that by using
memo
from React since after some inspection I noticed that some components were being re-rendered even when the props were not changing. This avoids that. For me, the performance improvement is pretty noticeable especially after walking through the app for some time.The memo just works after the second render so the first one does not change too much.
Another improvement that this PR does is regarding the
GamePage
.Instead of querying the GameInfo again, now the object is being passed from the
GameCard
, since it already has this information.Also, now the settings are stored using
electron-store
so we can share them easily.A good example is on the Download Dialog, where we were querying the settings just to know the default install path, causing an unnecessary re-render with that.
I did not dig deeply into these changes on Settings since would be too much for this PR, but we can start using it more from now on in future PRs.
For now, we are still storing the settings on both places to avoid breaking current Heroic users, after a few releases will be safe to remove the original approach.
Other than that I added some small fixes introduced on previous PRs like the issue with the ContextMenu on Gamecard and a blank library when clicking Back from Settings or GamePage.
Use the following Checklist if you have changed something on the Backend or Frontend: