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

Adds confirmation dialog before accepting a rerun #170

Merged
merged 2 commits into from
May 7, 2024

Conversation

omar-selo
Copy link
Collaborator

Resolves https://warthogs.atlassian.net/browse/RTW-306

Adds confirmation dialog before submitting a rerun request. This confirmation dialog can be accepted immediately by pressing enter.

Screencast.from.2024-05-06.12-58-59.webm

Copy link
Contributor

@andrejvelichkovski andrejvelichkovski left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Comment on lines +100 to +120
builder: (_) => AlertDialog(
title: const Text(
'Are you sure you want to rerun this test execution?',
),
actions: [
TextButton(
autofocus: true,
onPressed: () {
ref
.read(artefactBuildsProvider(artefactId).notifier)
.rerunTestExecution(testExecution.id);
context.pop();
},
child: const Text('yes'),
),
TextButton(
onPressed: () => context.pop(),
child: const Text('no'),
),
],
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can refactor this in a separate component, maybe a TestExecutionRerunAlertDialog to make the code a bit more structured?

Copy link
Collaborator Author

@omar-selo omar-selo May 7, 2024

Choose a reason for hiding this comment

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

I would love to. That's actually how I started, a separate widget. But for some reason this line of code builder: () => TestExecutionRerunAlertDialog kept failing and returning an error. Something about missing Navigator context in parent widget IIRC. Tried to get it to work but it wouldn't budge. So I resorted to this

@omar-selo omar-selo merged commit dc5b80b into main May 7, 2024
2 of 4 checks passed
@omar-selo omar-selo deleted the rerun-confirmation-dialog branch May 7, 2024 13:54
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