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.

Sign up for GitHub

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

Jump to bottom

Feature: Add flyout to confirm tag deletion #11358

Merged
merged 9 commits into from
Feb 20, 2023
Merged

Feature: Add flyout to confirm tag deletion #11358

merged 9 commits into from
Feb 20, 2023

Conversation

ferrariofilippo
Copy link
Contributor

@ferrariofilippo ferrariofilippo commented Feb 18, 2023 •
edited
Loading

Resolved / Related Issues
Items resolved / related issues by this PR.

Work in Progress

Validation
How did you test these changes?

  • Built and ran the app
  • Tested the changes for accessibility

Screenshots (optional)
1

@ferrariofilippo ferrariofilippo marked this pull request as draft February 18, 2023 19:10
@ferrariofilippo ferrariofilippo marked this pull request as ready for review February 19, 2023 20:53
@yaira2
Copy link
Member

yaira2 commented Feb 20, 2023

Can you add a cancel option?

@yaira2 yaira2 added the changes requested Changes are needed for this pull request label Feb 20, 2023
@yaira2
Copy link
Member

yaira2 commented Feb 20, 2023

Thanks! Can you udpate the screenshot?

@yaira2
Copy link
Member

yaira2 commented Feb 20, 2023

Is primary button supposed to be on the left?

@ferrariofilippo
Copy link
Contributor Author

I don't think so. It wouldn't be consistent with Cancel - SaveChanges buttons

@yaira2 yaira2 added needs - code review and removed changes requested Changes are needed for this pull request labels Feb 20, 2023
yaira2
yaira2 approved these changes Feb 20, 2023
View reviewed changes
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 changed the title Feature: Flyout to confirm tag deletion Feature: Add flyout to confirm tag deletion Feb 20, 2023
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Feb 20, 2023
@yaira2
Copy link
Member

yaira2 commented Feb 20, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yaira2 yaira2 merged commit c5ce77e into files-community:main Feb 20, 2023
@ferrariofilippo ferrariofilippo deleted the Tag_Confirmation branch February 20, 2023 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yaira2 yaira2 yaira2 approved these changes

Assignees
No one assigned
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature: Flyout to confirm tag deletion
2 participants
@ferrariofilippo @yaira2