Skip to content

Conversation

yaira2
Copy link
Member

@yaira2 yaira2 commented Sep 30, 2022

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

Validation
How did you test these changes?

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

Screenshots (optional)
Add screenshots here.

@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Sep 30, 2022
@yaira2 yaira2 changed the title Feature: Added setting to change default layout mode Feature: Added option to change default layout mode Sep 30, 2022
@yaira2 yaira2 changed the title Feature: Added option to change default layout mode Feature: Added option to change default layout mode Sep 30, 2022
@yaira2 yaira2 requested a review from gave92 September 30, 2022 05:03
@yaira2
Copy link
Member Author

yaira2 commented Oct 3, 2022

@ferrariofilippo can you look this over? I want to get it merged before #10117. This requires some stress testing, try to break it 🙂

@ferrariofilippo
Copy link
Contributor

I tried everything I thought of, and it seems the only bug is the one we already know.

I think the user should be able to decide even among grid layout sizes.
At the moment this feature uses the last size selected by the user when ForceOnAllDirectories is enabled.
If you want to reproduce:

  • Reset preferences
  • Enable ForceOnAllDirectories
  • Set grid layout size
  • Disable ForceOnAllDirectories
  • Change DefaultLayout to Grid

I think this should be changed because a user would need to do all the above only to change the default items' size for the grid layout.

@yaira2
Copy link
Member Author

yaira2 commented Oct 4, 2022

There are a range of sizes for the grid layouts, perhaps we can support a couple presets in the drop down (small, medium, large). I'm not sure how to do that right now as we don't have enums for those.

@ferrariofilippo
Copy link
Contributor

we can support a couple presets in the drop down (small, medium, large)

I think that's enough.

@yaira2 yaira2 merged commit 96384b2 into main Oct 6, 2022
@yaira2 yaira2 deleted the ya/DefaultLayoutMode branch October 6, 2022 14:59
@yaira2
Copy link
Member Author

yaira2 commented Oct 6, 2022

Merged this as is so that we can merge the default column options, should be okay if we add the options later on.

@ferrariofilippo
Copy link
Contributor

Yes, after all it's working with no bugs.
Choosing the size of GridView is a feature we can add later

if (!userSettingsService.PreferencesSettingsService.ForceLayoutPreferencesOnAllDirectories
&& folderSettings.IsAdaptiveLayoutEnabled
&& !folderSettings.IsLayoutModeFixed
&& folderSettings.LayoutMode == FolderLayoutModes.Adaptive)
Copy link
Member

@gave92 gave92 Oct 8, 2022

Choose a reason for hiding this comment

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

Note that this prevents adaptive layout mode from working because folderSettings.LayoutMode is never set to Adaptive. Even in adaptive mode folderSettings.LayoutMode is still either details or grid or tiles. The above check for IsAdaptiveLayoutEnabled is what we want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Pull requests that are ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Add option to choose the default layout mode
3 participants