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

feat: add uninstall button to store #739

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

alvaro-cuesta
Copy link
Contributor

@alvaro-cuesta alvaro-cuesta commented Jan 2, 2025

Please tick as appropriate:

  • I have tested this code on a steam deck or on a PC
  • My changes generate no new errors/warnings
  • This is a bugfix/hotfix
  • This is a new feature

Description

This is a followup to PR #733. After discussing with @TrainDoctor on Discord he suggested it would be a good idea to also add an uninstall button to the store next to the "Reinstall" button (to plugins which are already installed).

Screenshot

To make the functionality more obvious, the button will only be shown on the "Reinstall" case, i.e. when the currently installed version is selected in the dropdown.

@@ -32,7 +32,6 @@ const PluginCard: FC<PluginCardProps> = ({ storePlugin, installedPlugin }) => {

return (
<div
className="deckyStoreCard"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These classes seemed unused.

Copy link
Member

Choose a reason for hiding this comment

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

they exist for theme developers, keep them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AAGaming00 done. Also added a small warning documenting this.

@TrainDoctor TrainDoctor requested a review from a team January 16, 2025 18:54
@TrainDoctor TrainDoctor added the enhancement New feature or request label Jan 16, 2025
@TrainDoctor
Copy link
Member

@AAGaming00 need you to confirm any questions PR submitter had to make sure there aren't any issues with frontend changes made. Once the frontend changes have been reviewed I'll give this a spin myself.

@AAGaming00
Copy link
Member

AAGaming00 commented Jan 16, 2025

@alvaro-cuesta please revert 48f23f
the class names are there intentionally to aid theme devs

we should really document this practice...

Copy link
Member

@AAGaming00 AAGaming00 left a comment

Choose a reason for hiding this comment

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

see comment

Although the classes seemed to be unused, they exist for theme
developers.

Also add a warning documenting this.
Class was removed in ebc2f52 because it seemed no longer needed after
removing some styling, but it's actually there for theme developers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

3 participants