Skip to content

Fix: Fixed issue of overlapping brushes leaving gap in the Columns View #15096

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.

Sign up for GitHub

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
yaira2 merged 2 commits into from
Apr 1, 2024
Merged

Fix: Fixed issue of overlapping brushes leaving gap in the Columns View #15096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
465261b
Fix issue of overlapping brushes leaving gap on column layout
marcelwgn Apr 1, 2024
3c36d40
Update heights
yaira2 Apr 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/Files.App/Helpers/Layout/LayoutSizeKindHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,17 @@ public static int GetColumnsViewRowHeight(ColumnsViewSizeKind columnsViewSizeKin
switch (columnsViewSizeKind)
{
case ColumnsViewSizeKind.Compact:
return 28;
return 24;
case ColumnsViewSizeKind.Small:
return 36;
return 32;
case ColumnsViewSizeKind.Medium:
return 40;
return 36;
case ColumnsViewSizeKind.Large:
return 44;
return 40;
case ColumnsViewSizeKind.ExtraLarge:
return 48;
return 44;
default:
return 36;
return 32;
}
}

Expand Down
1 change: 1 addition & 0 deletions src/Files.App/Views/Layouts/ColumnLayoutPage.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@
<Grid
x:Name="FilesRootGrid"
Height="{Binding RowHeight, ElementName=PageRoot, Mode=OneWay}"
Margin="0,2,0,2"
Padding="12,0,12,0"
HorizontalAlignment="Stretch"
VerticalAlignment="Center"
Expand Down
21 changes: 15 additions & 6 deletions src/Files.App/Views/Layouts/ColumnLayoutPage.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ internal void ClearOpenedFolderSelectionIndicator()
return;

openedFolderPresenter.Background = new SolidColorBrush(Microsoft.UI.Colors.Transparent);
var presenter = openedFolderPresenter.FindDescendant<Grid>()!;
presenter!.Background = new SolidColorBrush(Microsoft.UI.Colors.Transparent);
SetFolderBackground(openedFolderPresenter, new SolidColorBrush(Microsoft.UI.Colors.Transparent));
openedFolderPresenter = null;
}

Expand Down Expand Up @@ -157,8 +156,8 @@ private void HighlightPathDirectory(ListViewBase sender, ContainerContentChangin
if (args.Item is ListedItem item && columnsOwner?.OwnerPath is string ownerPath
&& (ownerPath == item.ItemPath || ownerPath.StartsWith(item.ItemPath) && ownerPath[item.ItemPath.Length] is '/' or '\\'))
{
var presenter = args.ItemContainer.FindDescendant<Grid>()!;
presenter!.Background = this.Resources["ListViewItemBackgroundSelected"] as SolidColorBrush;
SetFolderBackground(args.ItemContainer as ListViewItem, this.Resources["ListViewItemBackgroundSelected"] as SolidColorBrush);
Copy link
Member

@yaira2 yaira2 Apr 1, 2024

Choose a reason for hiding this comment

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

Is there a way to make this theme aware to fix #12577?

Copy link
Contributor Author

@marcelwgn marcelwgn Apr 1, 2024

Choose a reason for hiding this comment

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

I think, the only option would be to listen to theme changes and based on that reapply the background. Unfortunately, there is no code behind way to create a ThemeResouce binding (at none that I am aware of).

Copy link
Member

@yaira2 yaira2 Apr 1, 2024

Choose a reason for hiding this comment

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

Is it worth adding logic for that or should we leave as is since restarting the app will use the correct theme?

Copy link
Contributor Author

@marcelwgn marcelwgn Apr 1, 2024

Choose a reason for hiding this comment

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

Kinda depends on how urgent a fix is in your opinion. On App startup, the theme should match OS theme however if you change it during runtime, we might not know it. Given we use this.Resources the layout might have picked up the OS theme quite late in the lifecycle though

Copy link
Member

@yaira2 yaira2 Apr 1, 2024

Choose a reason for hiding this comment

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

We've only had one bug report so it's not the highest priority, if it's a simple fix I guess we should move forward but otherwise it's not worth it if it requires a complex workaround.

Copy link
Contributor Author

@marcelwgn marcelwgn Apr 1, 2024

Choose a reason for hiding this comment

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

I think it's not too hard, we would probably only need an event listener for the actual theme changed event. I'll add a comment to the issue in case anyone else wants to pick it up (and make it easier for newcomers to contribute)

Copy link
Member

@yaira2 yaira2 Apr 1, 2024

Choose a reason for hiding this comment

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

Thanks!


openedFolderPresenter = FileList.ContainerFromItem(item) as ListViewItem;
FileList.ContainerContentChanging -= HighlightPathDirectory;
}
Expand Down Expand Up @@ -279,8 +278,7 @@ protected override void FileList_SelectionChanged(object sender, SelectionChange

if (e.RemovedItems.Count > 0 && openedFolderPresenter != null)
{
var presenter = openedFolderPresenter.FindDescendant<Grid>()!;
presenter!.Background = this.Resources["ListViewItemBackgroundSelected"] as SolidColorBrush;
SetFolderBackground(openedFolderPresenter, this.Resources["ListViewItemBackgroundSelected"] as SolidColorBrush);
}

if (SelectedItems?.Count == 1 && SelectedItem?.PrimaryItemAttribute is StorageItemTypes.Folder)
Expand Down Expand Up @@ -592,5 +590,16 @@ internal void ClearSelectionIndicator()
FileList.SelectedItem = null;
LockPreviewPaneContent = false;
}

private static void SetFolderBackground(ListViewItem? lvi, SolidColorBrush? backgroundColor)
{
if (lvi == null || backgroundColor == null) return;


if (lvi.FindDescendant<Grid>() is Grid presenter)
{
presenter.Background = backgroundColor;
}
}
}
}