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

Flush local mod list before moving to profile #1178

Merged
merged 1 commit into from
Jan 27, 2024

Conversation

anttimaki
Copy link
Collaborator

LocalModList is rendered before transition to the view actually takes place. If user has previously viewed a large profile, it's still stored in VueX and this unnecessary step can take several seconds. Resetting the stored mod list prevents this unnecessary wait.

Ideally this would be done when the mod list component unmounts, but that would mess with the NavigationBar which shows the number of installed mods.

LocalModList is rendered before transition to the view actually takes
place. If user has previously viewed a large profile, it's still stored
in VueX and this unnecessary step can take several seconds. Resetting
the stored mod list prevents this unnecessary wait.

Ideally this would be done when the mod list component unmounts, but
that would mess with the NavigationBar which shows the number of
installed mods.
@ebkr
Copy link
Owner

ebkr commented Jan 23, 2024

If it's better to do it when the mod list component unmounts, we could have a separate store which keeps track of the installed count. There is however potential for it to de-sync so may be iffy. Any thoughts?

@anttimaki
Copy link
Collaborator Author

If it's better to do it when the mod list component unmounts, we could have a separate store which keeps track of the installed count. There is however potential for it to de-sync so may be iffy. Any thoughts?

I played around with this idea a bit and it probably can be done, but it would a) temporarily show an outdated mod count from previously viewed profile when a new profile is opened, OR b) require updating the mod count when the NavigationLayout unmounts.

The latter would be fine except it's kind of detached from the rest of the implementation, location wise. Someone wondering why the number updates when it does probably wouldn't guess to check out NavigationLayout.vue? If we're okay with using that location, then it would probably make sense just use the unmounting of NavigationLayout to clear the local mod list directly. That way there wouldn't be a need for a separate store value.

Copy link
Collaborator

@MythicManiac MythicManiac left a comment

Choose a reason for hiding this comment

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

This has a minor chance for a race condition to occur between the two awaits, but I'm quite sure it shouldn't be a significant problem even if that happens.

I do agree there's probably a better way to do this, but given that this change shouldn't be harmful and it is an overall improvement, I'd say this can be merged as is & if improvement is done (e.g. by moving this to the unmount), we can have a separate PR for that.

I'm assuming @ebkr you're fine with the above since you didn't explicitly reject this one, so I'll merge this as-is

@MythicManiac MythicManiac merged commit e95f3c3 into develop Jan 27, 2024
3 checks passed
@MythicManiac MythicManiac deleted the flush-modlist-before-opening-profile branch January 27, 2024 01:57
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.

3 participants