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

[Linux] Added checkbox to uninstall-modal to remove prefix aswell #940

Merged
merged 8 commits into from
Feb 1, 2022

Conversation

Nocccer
Copy link
Collaborator

@Nocccer Nocccer commented Jan 30, 2022

Added checkbox to uninstall modal to choose if prefix should be removed aswell.
Checkbox is checked by default.
image


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)

@Nocccer Nocccer requested a review from flavioislima January 30, 2022 17:53
@Nocccer Nocccer changed the title [General] Added checkbox to uninstall modal to remove prefix aswell [General] Added checkbox to uninstall-modal to remove prefix aswell Jan 30, 2022
@Nocccer Nocccer changed the title [General] Added checkbox to uninstall-modal to remove prefix aswell [Linux] Added checkbox to uninstall-modal to remove prefix aswell Jan 30, 2022
@flavioislima
Copy link
Member

Looks good. But I think Uninstall Prefix is not the right way of saying it. Perhaps Removing the prefix instead.
Like:
Would you like to remove the Prefix: path/to/prefix as well? This can't be undone.
what do you think?

@Nocccer
Copy link
Collaborator Author

Nocccer commented Jan 31, 2022

Looks good. But I think Uninstall Prefix is not the right way of saying it. Perhaps Removing the prefix instead. Like: Would you like to remove the Prefix: path/to/prefix as well? This can't be undone. what do you think?

Sure. "Uninstall prefix aswell" is only the default value for i18n translation

@flavioislima
Copy link
Member

Sure. "Uninstall prefix aswell" is only the default value for i18n translation

Yes but in this case, all translations will be based on that.

@Nocccer Nocccer force-pushed the delete-prefix-on-uninstall branch from e2298ea to ff906ec Compare February 1, 2022 13:11
@Nocccer
Copy link
Collaborator Author

Nocccer commented Feb 1, 2022

@flavioislima added the suggested message

src/helpers/library.ts Outdated Show resolved Hide resolved
Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

I think only one last thing that I missed on first review. Leave that checkbox unchecked by default because some people have one prefix for several games. I do that sometimes and keep specific prefixes only for specific games.

@Nocccer Nocccer merged commit 1e1be59 into main Feb 1, 2022
@Nocccer Nocccer deleted the delete-prefix-on-uninstall branch February 1, 2022 17:02
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
2 participants