[#60588] Implement Danger Dialog warning variant, without second step confirmation #234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: af6ae73 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Changes preview to use `primer_form_with` and an example `DeletionForm`. This provides a more realistic example of an advanced integration with form builders.
(tldr: not to be confused with `within`) Only other assertions should be called within `&optional_filter_block`: this filter block is passed to `Capybara::SelectorQuery`, which resets the wait time to 0 before calling it. Actions such as `fill_in` may rely on a longer wait time - this is the case in particular for our overriden `fill_in` (see `System::TestCase`), which executes the Axe accessibility testing engine asynchronously after a text field is filled in.
myabc
force-pushed
the
feature/60588-danger-warning-dialog
branch
from
eab7b13 to
2214d56
Compare
myabc
changed the title
Applies `display: contents` style to `danger-dialog-form-helper`, which makes the custom element behave as if it doesn't exist in the DOM structure, passing all styles directly to its children. This makes `scrollable-region` behaves correcly, which in turn ensures the dialog behaves the same as `Primer::Alpha::Dialog` with regards to scrolling.
myabc
force-pushed
the
feature/60588-danger-warning-dialog
branch
from
5e8a646 to
10e03e7
Compare
6 tasks
Changes the default confirm button text from "Delete permanently" to the less-destructive "Delete".
Merged
3 tasks
10 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
Implementation of Danger Dialog, a generalised dialog for "potentially dangerous" actions such as item deletion.
There are two variants:
Screenshots
Integration
All references in calling code to
DangerConfirmationDialogwill need to be updated toDangerDialog.Callers will need to update the class name and consider which behaviour they wish to present to the end user. The confirmation variant behaviour is enabled by defining a
confirmation_check_boxslot.List the issues that this change affects.
https://community.openproject.org/wp/60588
Risk Assessment
What approach did you choose and why?
The fewest code changes necessary.
Anything you want to highlight for special attention from reviewers?
In summary this PR:
DangerConfirmationDialogtoDangerDialog.confirmation_check_boxslot optional. If this slot is not provided, the confirmation button is always enabled.This PR also
Accessibility
Removing the check box control from
scrollable-regionuncovered a Axe violation (in Firefox only). This was fixed in commit 10e03e7.Merge checklist