Skip to content

[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

Merged
merged 19 commits into from
Nov 14, 2022

Conversation

flavioislima
Copy link
Member

@flavioislima flavioislima commented Nov 8, 2022

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:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@flavioislima flavioislima requested review from a team, arielj, CommandMC, Nocccer, imLinguin and redromnon and removed request for a team November 8, 2022 19:53
@flavioislima flavioislima added the pr:ready-for-review Feature-complete, ready for the grind! :P label Nov 8, 2022
@@ -1,2 +1,2 @@
#!/bin/bash
yarn codecheck && yarn lint && yarn prettier-fix && yarn i18n
Copy link
Member Author

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.

@arielj
Copy link
Collaborator

arielj commented Nov 9, 2022

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.

@flavioislima
Copy link
Member Author

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.

@flavioislima flavioislima merged commit 86e2b45 into beta Nov 14, 2022
@flavioislima flavioislima deleted the other/render_optimizations branch November 14, 2022 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants