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

Fix accidental dismissal of MultipleProductKey dialog #2109

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

loklaan
Copy link

@loklaan loklaan commented Jan 4, 2025

Overview 📄

Hey folks, this PR aims to improve the experience of user input when filling out Multiple Product Keys, by turning off dialog dismissals that could happen accidentally.

This came out of my own "Oh no, all my keys!" moment, when I accidentally clicked outside of the dialog.

Problem 🤔

The "Multiple Product Keys" dialog The clickable "outside" element

The following are user interactions to dismiss the Multple Product Keys dialog, and the changes made to each

🖱️ Mouse

  1. Clicking the "Close" button inside of the dialog.
    👍 No changes.
  2. Clicking outside of the dialog (e.g. in the red shown above).
    💥 This has now been disabled.

⌨️ Keyboard

  1. Pressing Escape on the keyboard.
    💥 This has now been disabled.

The latter two user interactions can be awkward and were at fault for me personally, so I'm got rid of 'em.

Ideally, an alert of some kind like "You have unsaved Product Keys" canceable alert shown before dialog dismissal would be the solution I'd aim for, but I'm unfamiliar with the codebase and the solution in this PR seemed a reasonable shortcut.

Solution 🚀

There is a bExplicitDismissalOnly param that can be passed to the underlying CModal implementation in the SteamScriplet, this takes advantage of that! It's already in use in this extension, so I reckon it's a safe usage.

@candela97
Copy link
Collaborator

I'm working on overhauling this feature, and will include this improvement. But yeah this is what's needed.

@loklaan
Copy link
Author

loklaan commented Jan 17, 2025

I'm working on overhauling this feature, and will include this improvement. But yeah this is what's needed.

That's great to hear!! Actively as of now? Shout if you'd like a friendly QA to test run 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