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

Continue refactoring ProfileImportModal to eventually separate business logic from UI #1477

Open
wants to merge 10 commits into
base: immutable-profile-pt6-profileinstallprovider-install
Choose a base branch
from

Conversation

anttimaki
Copy link
Collaborator

No description provided.

Avoid unnecessary non-null assertions as that's a bad habit. Throw an
error if components local state is invalid before starting the import.
This makes the code easier to read and might make it easier to refactor
the profileCreatedCallback further.
Profiles imported as code are always .r2z files. Previously user could
choose the older .r2x format when importing profile from a file, but
support for this was dropped recently, and only .r2z is supported now.
As a bonus this now handless the unlikely error properly instead of
just ignoring it silently.
This requires slight changes to addEventListener, but these changes are
probably an improvements by themselves.
- Earlier step has checked there's importable mods in the profile
- Download provider has another, less user-friendly check for the same
  thing to prevent the mod profile from getting stuck
- It doesn't seem anything catastrophic would happen even if the rest
  of the code is executed using an empty mod list
I don't see any reason for to wrap the code in the timeout. This might
have been relevant when the import step was started right away after a
new profile was created, by there now exists a preview window between
these steps so the folder should always exists. If it doesn't, the
100ms wait won't change things.
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