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

Change rerun button order #176

Merged
merged 2 commits into from
May 8, 2024
Merged

Conversation

andrejvelichkovski
Copy link
Contributor

This small change moves the rerun button from the left of the environment name to the right handside, just before the test execution review button.

This was suggested as a user experience improvement.

Screenshot

Screenshot from 2024-05-08 12-35-58

Comment on lines 60 to 61
const SizedBox(width: Spacing.level2),
_RerunButton(testExecution: testExecution),
const SizedBox(width: Spacing.level2),
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps we could either merge the SizedBox or remove one of them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like it was level4 before adding the Rerun button here, so I decided to restore it to level 4 here

Comment on lines 67 to 68
_RerunButton(testExecution: testExecution),
const SizedBox(width: Spacing.level4),
Copy link
Collaborator

Choose a reason for hiding this comment

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

the screenshot you supplied shows more spacing between rerun and signoff button than between signoff button and CI link. Perhaps we can reduce this spacing?

Copy link
Contributor Author

@andrejvelichkovski andrejvelichkovski May 8, 2024

Choose a reason for hiding this comment

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

Which one do you think fits best

spacing level 3

spacing-3

spacing level 2

spacing-2

Copy link
Collaborator

Choose a reason for hiding this comment

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

3 seems to strike a balance

Copy link
Collaborator

@omar-selo omar-selo 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!

@andrejvelichkovski andrejvelichkovski merged commit 45cbde1 into main May 8, 2024
2 of 3 checks passed
@andrejvelichkovski andrejvelichkovski deleted the change-rerun-button-order branch May 8, 2024 11:52
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