Skip to content

Implement folder setting to sort directories and files together and add option to toolbar sort flyout #9098

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

Merged
merged 9 commits into from
May 12, 2022

Conversation

manfromarce
Copy link
Contributor

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

Details of Changes

  • The global setting to sort directories alongside files is removed and the option is loaded/saved per-folder (like "sort by" and ascending/descending)
  • The option is added to the toolbar flyout

Validation
How did you test these changes?

  • Built and ran the app

Screenshots (optional)
immagine

manfromarce and others added 3 commits May 6, 2022 19:42
Co-authored-by: Yair Aichenbaum <39923744+yaichenbaum@users.noreply.github.com>
@manfromarce
Copy link
Contributor Author

I've restored the global setting and it seems to work, except that already open tabs are not updated when the setting is changed.

@yaira2
Copy link
Member

yaira2 commented May 8, 2022

I've restored the global setting and it seems to work, except that already open tabs are not updated when the setting is changed.

Is this somethign you can copy from the option to show/hide hidden items?

@manfromarce
Copy link
Contributor Author

The problem is that for each folder visited if there is no custom setting, the value of the global setting is saved. So when the user changes the global setting again it will have no effect on that folder

@yaira2
Copy link
Member

yaira2 commented May 8, 2022

The problem is that for each folder visited if there is no custom setting, the value of the global setting is saved. So when the user changes the global setting again it will have no effect on that folder

@manfromarce that's expected, to have the setting apply on all items you need to turn off the option to use individual preferences.

@manfromarce
Copy link
Contributor Author

Ok, I've done some tests and the combination of global and individual settings seems to work properly

@yaira2 yaira2 requested a review from gave92 May 9, 2022 19:42
@yaira2
Copy link
Member

yaira2 commented May 9, 2022

Ok, I've done some tests and the combination of global and individual settings seems to work properly

Awesome! Do you think the option to set the default value should be disabled when the option to use individual preferences is turned off?

@gave92
Copy link
Member

gave92 commented May 9, 2022

Ok, I've done some tests and the combination of global and individual settings seems to work properly

Awesome! Do you think the option to set the default value should be disabled when the option to use individual preferences is turned off?

Alternatives:

  • the two options should synced <- preferred?
  • the new option in the toolbar should be hidden

@manfromarce
Copy link
Contributor Author

It could be that when individual settings are disabled the new toolbar command is synced with the general setting (like show hidden/dot files)

@yaira2
Copy link
Member

yaira2 commented May 9, 2022

It could be that when individual settings are disabled the new toolbar command is synced with the general setting (like show hidden/dot files)

That can work, we can change it later if it gets confusing.

@manfromarce
Copy link
Contributor Author

@yaichenbaum @gave92 The settings should now be linked and refreshed correctly when individual settings are disabled, let me know what you think

@yaira2
Copy link
Member

yaira2 commented May 12, 2022

@manfromarce it's working great! We'll probably have to rearrange some options so that it's easier to see what's going on, but this is fantastic.
Do you want to add a setting to choose the default layout and default sort mode using the same logic?

yaira2
yaira2 previously approved these changes May 12, 2022
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

@manfromarce
Copy link
Contributor Author

OK, I think I can implement them. We could maybe add a new group in settings because "files and folders" would have too many entries

@yaira2
Copy link
Member

yaira2 commented May 12, 2022

OK, I think I can implement them. We could maybe add a new group in settings because "files and folders" would have too many entries

We can add a group for sorting preferences

@gave92 gave92 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels May 12, 2022
@yaira2 yaira2 merged commit 3fcea26 into files-community:main May 12, 2022
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.

Add option to sort flyout to sort files and folders together
3 participants