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

Remove draggable from Run EXE on Prefix button #1907

Merged
merged 1 commit into from Oct 17, 2022
Merged

Conversation

arielj
Copy link
Collaborator

@arielj arielj commented Oct 15, 2022

This PR removes the draggable attribute of the button. The button supports dragging and dropping a file ON the button, but the button itself does not need to be draggable.

I think this is causing issues when clicking that button on the Steam Deck (it might be trying to trigger the drag instead of clicking it). I'm not sure how to test this directly on the device though but it's the only difference I can see comparing this button with the other buttons that open a file picker dialog.

Click and dropping files on the button still work fine after this change.

The returns were not needed


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)

@arielj arielj requested review from a team, flavioislima, CommandMC, Nocccer, imLinguin and redromnon and removed request for a team October 15, 2022 18:59
Copy link
Collaborator

@CommandMC CommandMC left a comment

Choose a reason for hiding this comment

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

According to the MDN web docs, just adding draggable without setting it to a value will not actually do anything, so I doubt this will change anything at all, but it's still good to remove an unneeded property

@arielj
Copy link
Collaborator Author

arielj commented Oct 15, 2022

According to the MDN web docs, just adding draggable without setting it to a value will not actually do anything, so I doubt this will change anything at all, but it's still good to remove an unneeded property

The generated HTML was draggable="true" and it was actually draggable. I imagine JSX was doing something fishy there.

@arielj arielj added the pr:ready-for-review Feature-complete, ready for the grind! :P label Oct 15, 2022
@flavioislima
Copy link
Member

I think we should remove this feature and just add a new button like the others. This dragable feature is not really used and breaks the consistency of the settings page imo.

@arielj
Copy link
Collaborator Author

arielj commented Oct 16, 2022

I think we should remove this feature and just add a new button like the others. This dragable feature is not really used and breaks the consistency of the settings page imo.

It also works as a button though (it's just broken on the steam deck for me, but works fine with a click on desktop).

EDIT: I also think we should remove the feature to drop files in that button though, I just don't know what was the reason it was added and I don't know if users actually use it

@arielj
Copy link
Collaborator Author

arielj commented Oct 17, 2022

I'm merging this, I think moving this button to be a different thing would be better done in another PR since it will have to change design and functionality.

@arielj arielj merged commit aaefda2 into beta Oct 17, 2022
@arielj arielj deleted the fixes/run-exe-on-prefix branch October 17, 2022 03:03
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.

None yet

3 participants