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: Limit permanent warning dismissal to affirmative actions #292

Merged

Conversation

dcalhoun
Copy link
Member

@dcalhoun dcalhoun commented Jun 24, 2024

Related to #287 (comment).

Proposed Changes

feat: Limit permanent warning dismissal to affirmative actions
Permanently dismissing a warning while selecting the "cancel" option is
confusing as generally cancelling a dialog does not persist selections.

feat: Rephrase download optimized version cancel option
This option is not so much a cancellation, but either a deferment of a
highly-recommended action or an acceptance of actively choosing to not
heed direct advice for solving an system issue. Rephrasing the option to
align with this better communicates the choice and makes the permanent
dismissal option more understandable.

Testing Instructions

Verify permanent dismissal is disallowed when selecting the "cancel" option

  1. Trigger the display of a warning dialog.
  2. Check the "Don't show this warning again" option.
  3. Click the "cancel" option.
  4. Verify the dialog is displayed after triggering the relevant scenario (step
    1).

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

Footnotes

  1. There is no need to complete all of the linked testing steps, only those related for triggering the relevant warning dialog. 2 3

Permanently dismissing a warning while selecting the "cancel" option is
confusing as generally cancelling a dialog does not persist selections.
This option is not so much a cancellation, but either a deferment of a
highly-recommended action or an acceptance of actively choosing to not
heed direct advice for solving an system issue. Rephrasing the option to
align with this better communicates the choice and makes the permanent
dismissal option more understandable.
@dcalhoun dcalhoun added the [Type] Enhancement Improvement upon an existing feature label Jun 24, 2024
@dcalhoun dcalhoun self-assigned this Jun 24, 2024
@dcalhoun dcalhoun marked this pull request as ready for review June 24, 2024 13:17
@dcalhoun dcalhoun requested review from matt-west and a team June 24, 2024 13:18
Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

LGTM 🎊 !

@dcalhoun dcalhoun merged commit f876bca into trunk Jun 25, 2024
16 checks passed
@dcalhoun dcalhoun deleted the feat/limit-permanent-warning-dismissal-to-affirmative-actions branch June 25, 2024 12:18
fluiddot pushed a commit that referenced this pull request Jun 27, 2024
* feat: Limit permanent warning dismissal to affirmative actions

Permanently dismissing a warning while selecting the "cancel" option is
confusing as generally cancelling a dialog does not persist selections.

* feat: Rephrase download optimized version cancel option

This option is not so much a cancellation, but either a deferment of a
highly-recommended action or an acceptance of actively choosing to not
heed direct advice for solving an system issue. Rephrasing the option to
align with this better communicates the choice and makes the permanent
dismissal option more understandable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement Improvement upon an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants