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

Added context menu options to rearrange Favorites item #5979

Merged
merged 12 commits into from
Aug 31, 2021

Conversation

BanCrash
Copy link
Contributor

@BanCrash BanCrash commented Aug 30, 2021

Opening this as a draft since there is still a visual bug that I didn't figure out how to fix it.

I will try to solve this, but since the timeline for the V2 is almost done I suggest to set a timeline also for this, and if I can't solve this issue since it's only visual merge this and open a new issue to fix it in the future.

The issue is that when you move the actual selected item, it will stay as selected even if you select another one: partially fixed, now it will work as expected except that moved item won't have the selection pill.

image

Resolved / Related Issues

Details of Changes

  • Added context menu options to rearrange Favorites item (right click on an item to have to option to move it up/down/top/bottom).

Current limitations

  • Not available for Home nor Recycle bin. Maybe in a future PR.
  • There is a bug when you move the actual selected item, that it will keep it selected even if you switch to another selected item until you select it back again. There is a bug that doesn't show the selection pill when you change the actual selected item.

Validation
How did you test these changes?

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

Screenshots (optional)
image

GIF 30-08-2021 15-49-29

Remaining:
- Not show unpertinent options (move to top / move up for the first, move to bottom / move down to the last, etc).
- Add support for rearranging recycle bin.
Remaining:
- Selection is not working if you move the item you have selected on the sidebar.

Will not be done at the moment:
- Move recycle bin and Home.
Move part of SwapItems method code to MoveItem method so it could be used for both drag and drop (when availabe) and context menu.
@BanCrash
Copy link
Contributor Author

I think the bug I've found it's not from Files but from WinUi. I've found this similar issue, though not exactly the same:
microsoft/microsoft-ui-xaml#2845

And also this that I'm not sure neither if it's the same: microsoft/microsoft-ui-xaml#3015

@BanCrash BanCrash marked this pull request as ready for review August 30, 2021 19:48
Files/UserControls/SidebarControl.xaml.cs Outdated Show resolved Hide resolved
Co-authored-by: Yair Aichenbaum <39923744+yaichenbaum@users.noreply.github.com>
@R3voA3
Copy link
Contributor

R3voA3 commented Aug 30, 2021

While I like the idea of being able to change the order of appearance, wouldn't it be more intuitive to just be able to drag and drop the items? With the context menu, rearranging many items can become tedious.

@BanCrash
Copy link
Contributor Author

BanCrash commented Aug 30, 2021

While I like the idea of being able to change the order of appearance, wouldn't it be more intuitive to just be able to drag and drop the items? With the context menu, rearranging many items can become tedious.

My idea would be to have both ways and everyone choose whatever option they want (or even sometimes one and sometimes the other). But currently there is a bug on WinUI that doesn't allow to have the drag and drop functionality. When that bug is fixed we can have also that way.

@R3voA3
Copy link
Contributor

R3voA3 commented Aug 30, 2021

While I like the idea of being able to change the order of appearance, wouldn't it be more intuitive to just be able to drag and drop the items? With the context menu, rearranging many items can become tedious.

My idea would be to have both ways and everyone choose whatever option they want (or even sometimes one and sometimes the other). But currently there is a bug on WinUI that doesn't allow to have the drag and drop functionality. When that bug is fixed we can have also that way.

Sounds great!

@yaira2
Copy link
Member

yaira2 commented Aug 30, 2021

While I like the idea of being able to change the order of appearance, wouldn't it be more intuitive to just be able to drag and drop the items? With the context menu, rearranging many items can become tedious.

As @BanCrash mentioned, drag and drop would be ideal, but until the WinUI issue is resolved, we're implementing this as a workaround.

Copy link
Member

@gave92 gave92 left a comment

Choose a reason for hiding this comment

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

Nice!

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Aug 31, 2021
@yaira2 yaira2 self-requested a review August 31, 2021 17:07
@yaira2 yaira2 merged commit 4c363db into files-community:main Aug 31, 2021
@BanCrash BanCrash deleted the favoritesOrder branch September 1, 2021 06:56
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.

4 participants