Feature: Added slider to control item size (phase 1) #14719

yaira2 · 2024-02-14T03:22:05Z

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.
For Feature: Slider to control thumbnail size #10492...

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. Confirm view type can still be overridden in folders (unless syncing is enabled)
2. Confirm the view size syncs between all folders (even when syncing is turned off)
3. Confirm each view type remembers it's size
4. Confirm keyboard shortcuts to change view & view size are working
5. Confirm size slider is working

Screenshots (optional)
Add screenshots here.

yaira2 · 2024-02-14T04:34:03Z

src/Files.App/Actions/Display/LayoutAction.cs

Note to update docs

src/Files.App/Helpers/Layout/LayoutPreferencesManager.cs

src/Files.App/Data/Models/ItemViewModel.cs

Aghyads · 2024-02-14T18:05:53Z

Sorry i'm new hear. So this is still in progress, not yet done to use, right?

yaira2 · 2024-02-14T18:17:09Z

@Aghyads this pull request adds the requested feature, there is still work to be done but the basic implementation is almost complete.

hishitetsu · 2024-02-15T06:16:48Z

src/Files.App/Actions/Display/LayoutAction.cs

@@ -206,6 +170,7 @@ protected virtual void OnContextChanged(string propertyName)

 	internal class LayoutDecreaseSizeAction : IAction


IsExecutable should be implemented.

hishitetsu · 2024-02-15T06:17:03Z

src/Files.App/Actions/Display/LayoutAction.cs

+					if (UserSettingsService.LayoutSettingsService.ItemSizeColumnsView > Constants.IconHeights.ColumnsView.Minimum)
+						UserSettingsService.LayoutSettingsService.ItemSizeColumnsView -= Constants.IconHeights.ColumnsView.Increment;
+					break;
+			}

 			return Task.CompletedTask;
 		}
 	}

 	internal class LayoutIncreaseSizeAction : IAction


hishitetsu · 2024-02-15T06:21:46Z

src/Files.App/Strings/en-US/Resources.resw

The values of LayoutDecreaseSizeDescription and LayoutIncreaseSizeDescription should be changed because their messages are currently limited to the grid layout.

hishitetsu · 2024-02-16T16:06:10Z

src/Files.App/Actions/Display/LayoutAction.cs

+		public bool IsExecutable
+			=> contentPageContext.PageType is not ContentPageTypes.Home;


IsExecutable should be false when ExecuteAsync does nothing.

We can add that but it's a lot of additional logic with little benefit.

It prevents meaningless commands from appearing in the command palette.

0x5bfa

✅ Tested locally; works for me.

0x5bfa · 2024-02-18T03:58:08Z

src/Files.App/Actions/Display/LayoutAction.cs

+		private static readonly IUserSettingsService UserSettingsService = Ioc.Default.GetRequiredService<IUserSettingsService>();
+		private readonly IDisplayPageContext displayPageContext = Ioc.Default.GetRequiredService<IDisplayPageContext>();
+		private readonly IContentPageContext contentPageContext = Ioc.Default.GetRequiredService<IContentPageContext>();


Should all be private properties with pascal case.

0x5bfa · 2024-02-18T03:58:20Z

src/Files.App/Actions/Display/LayoutAction.cs

+		private static readonly IUserSettingsService UserSettingsService = Ioc.Default.GetRequiredService<IUserSettingsService>();
+		private readonly IDisplayPageContext displayPageContext = Ioc.Default.GetRequiredService<IDisplayPageContext>();
+		private readonly IContentPageContext contentPageContext = Ioc.Default.GetRequiredService<IContentPageContext>();


0x5bfa · 2024-02-18T04:01:14Z

src/Files.App/Views/Layouts/DetailsLayoutPage.xaml.cs

+				if (value != UserSettingsService.LayoutSettingsService.ItemSizeDetailsView)
+				{
+					NotifyPropertyChanged(nameof(ItemSize));
+				}


Remove brackets

hishitetsu · 2024-02-20T01:59:52Z

I tried to test, but #14785 prevents me from testing properly.

yaira2 marked this pull request as draft February 14, 2024 03:22

yaira2 force-pushed the ya/DetailsHeight branch 3 times, most recently from 8eb9f21 to 3f9d349 Compare February 14, 2024 04:30

yaira2 commented Feb 14, 2024

View reviewed changes

src/Files.App/Actions/Display/LayoutAction.cs

Copy link

Member Author

yaira2 Feb 14, 2024

Choose a reason for hiding this comment

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

Note to update docs

yaira2 commented Feb 14, 2024

View reviewed changes

src/Files.App/Helpers/Layout/LayoutPreferencesManager.cs Outdated Show resolved Hide resolved

yaira2 force-pushed the ya/DetailsHeight branch from 2cac045 to d94cfc8 Compare February 14, 2024 05:07

yaira2 marked this pull request as ready for review February 14, 2024 05:10

yaira2 changed the title ~~Feature: Item size slider~~ Feature: Added slider to control item size Feb 14, 2024

yaira2 added the needs - code review label Feb 14, 2024

yaira2 force-pushed the ya/DetailsHeight branch from 943a202 to 3b4a334 Compare February 14, 2024 05:20

yaira2 requested a review from 0x5bfa February 14, 2024 05:36

This comment was marked as outdated.

Sign in to view

yaira2 commented Feb 14, 2024

View reviewed changes

src/Files.App/Data/Models/ItemViewModel.cs Outdated Show resolved Hide resolved

yaira2 force-pushed the ya/DetailsHeight branch from ecc6e7e to 226eadd Compare February 14, 2024 18:27

yaira2 requested a review from 0x5bfa February 14, 2024 19:41

yaira2 force-pushed the ya/DetailsHeight branch 3 times, most recently from 9949496 to e53df41 Compare February 15, 2024 04:18

hishitetsu reviewed Feb 15, 2024

View reviewed changes

yaira2 force-pushed the ya/DetailsHeight branch 2 times, most recently from e039d09 to f6ce634 Compare February 15, 2024 19:09

yaira2 changed the title ~~Feature: Added slider to control item size~~ Feature: Added slider to control item size (phase 1) Feb 15, 2024

yaira2 force-pushed the ya/DetailsHeight branch from f6ce634 to f10c9f2 Compare February 15, 2024 21:45

hishitetsu reviewed Feb 16, 2024

View reviewed changes

yaira2 requested a review from hishitetsu February 18, 2024 03:26

0x5bfa reviewed Feb 18, 2024

View reviewed changes

yaira2 force-pushed the ya/DetailsHeight branch from dafca55 to c1a59db Compare February 19, 2024 16:26

yaira2 force-pushed the ya/DetailsHeight branch from d243474 to 134e085 Compare February 19, 2024 23:33

Phase 1

80b889a

yaira2 force-pushed the ya/DetailsHeight branch from 0f5bf11 to 80b889a Compare February 20, 2024 00:11

Update GridLayoutPage.xaml

a161436

hishitetsu approved these changes Feb 20, 2024

View reviewed changes

yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Feb 20, 2024

yaira2 merged commit 4c40a7d into main Feb 20, 2024
6 checks passed

yaira2 deleted the ya/DetailsHeight branch February 20, 2024 16:00

yaira2 added a commit that referenced this pull request Feb 21, 2024

Feature: Added slider to control item size (phase 1) (#14719)

25fd021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Added slider to control item size (phase 1) #14719

Feature: Added slider to control item size (phase 1) #14719

yaira2 commented Feb 14, 2024 •

edited

Loading

yaira2 Feb 14, 2024

This comment was marked as outdated.

Aghyads commented Feb 14, 2024

yaira2 commented Feb 14, 2024

hishitetsu Feb 15, 2024

yaira2 Feb 15, 2024

hishitetsu Feb 15, 2024

yaira2 Feb 15, 2024

hishitetsu Feb 15, 2024

yaira2 Feb 15, 2024

hishitetsu Feb 16, 2024

yaira2 Feb 16, 2024 •

edited

Loading

hishitetsu Feb 16, 2024

0x5bfa left a comment

0x5bfa Feb 18, 2024

yaira2 Feb 19, 2024

0x5bfa Feb 18, 2024

yaira2 Feb 19, 2024

0x5bfa Feb 18, 2024

yaira2 Feb 19, 2024

hishitetsu commented Feb 20, 2024

		@@ -206,6 +170,7 @@ protected virtual void OnContextChanged(string propertyName)

		internal class LayoutDecreaseSizeAction : IAction

		public bool IsExecutable
		=> contentPageContext.PageType is not ContentPageTypes.Home;

Feature: Added slider to control item size (phase 1) #14719

Feature: Added slider to control item size (phase 1) #14719

Conversation

yaira2 commented Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

This comment was marked as outdated.

Aghyads commented Feb 14, 2024

yaira2 commented Feb 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaira2 Feb 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0x5bfa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hishitetsu commented Feb 20, 2024

yaira2 commented Feb 14, 2024 •

edited

Loading

yaira2 Feb 16, 2024 •

edited

Loading