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

Feature: Updated the design of the Tags section in the Details Pane #13138

Merged
merged 2 commits into from
Aug 7, 2023

Conversation

yaira2
Copy link
Member

@yaira2 yaira2 commented Aug 6, 2023

Resolved / Related Issues

  • Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
    Related to Feature: Add edit tags button to the preview pane #12946

Validation
How did you test these changes?

  • Did you build the app and test your changes?
  • Did you check for accessibility? You can use Accessibility Insights for this.
  • Did you remove any strings from the en-us resource file?
    • Did you search the solution to see if the string is still being used?
  • Did you implement any design changes to an existing feature?
    • Was this change approved?
  • Are there any other steps that were used to validate these changes?
    1. Open details pane
    2. Edit the tags using the menu flyout

Screenshots (optional)
image

@yaira2
Copy link
Member Author

yaira2 commented Aug 6, 2023

@ferrariofilippo fyi

@Lukiluc29
Copy link
Contributor

Lukiluc29 commented Aug 6, 2023

I'm sorry for intruding on your design update, but is it possible to fix the issue where we have to go back to the file/folder to see the tags?
Capture d'écran 2023-08-06 180840
Capture d'écran 2023-08-06 180808

@yaira2
Copy link
Member Author

yaira2 commented Aug 6, 2023

I'm sorry for intruding on your design update, but is it possible to fix the issue where we have to go back to the file/folder to see the tags?

Can you open an issue for this?

@ferrariofilippo
Copy link
Contributor

Can you open an issue for this?

Maybe it's something with this PR because it's working in Preview

@yaira2
Copy link
Member Author

yaira2 commented Aug 6, 2023

@ferrariofilippo what are the steps to repro the issue?

@ferrariofilippo
Copy link
Contributor

ferrariofilippo commented Aug 6, 2023

image

You're not updating the flyout when the selected item changes
(Screenshots are from an older commit)

@yaira2 line 53 shold fix all the issues

@ferrariofilippo
Copy link
Contributor

@ferrariofilippo what are the steps to repro the issue?

Open the preview pane select a folder, change the selected folder and try to edit tags

@Lukiluc29
Copy link
Contributor

I even found another issue! When I add a tag using the new button, the selected label is not assigned to the folder itself, but instead to the "Documents" folder in my case. However, when I click on another folder, like "Images" for example, the tag I selected for the previous folder is still selected.
Capture d'écran 2023-08-06 181649
Capture d'écran 2023-08-06 181936

@Lukiluc29
Copy link
Contributor

To summarize, all the tags that I apply, regardless of their location or the folder they are in, are associated with the "Documents" folder.

@ferrariofilippo
Copy link
Contributor

ferrariofilippo commented Aug 6, 2023

To summarize, all the tags that I apply, regardless of their location or the folder they are in, are associated with the "Documents" folder.

The screenshot I posted above fixes all the issues

@Lukiluc29
Copy link
Contributor

The screenshot I posted above fixes all the issues

All right

@Lukiluc29
Copy link
Contributor

I understand that I may be bothering you a bit, but I was wondering if it would be possible to decrease the space between each tag line. This would help save some space, especially when there are numerous tags (as shown in the example below).
Capture d'écran 2023-08-06 194355

@yaira2 yaira2 force-pushed the ya/DetailsPaneTags branch from 090cbf7 to bb62a4a Compare August 6, 2023 18:15
Copy link
Contributor

@ferrariofilippo ferrariofilippo left a comment

Choose a reason for hiding this comment

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

LGTM and works for me!

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Aug 7, 2023
@yaira2 yaira2 merged commit 6194696 into main Aug 7, 2023
@yaira2 yaira2 deleted the ya/DetailsPaneTags branch August 13, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants