Skip to content

Conversation

Dylan-Osborne
Copy link
Contributor

Added the "Tiles" layout based on File Explorer (Start to #753)
The tiles layout also supports displaying and editing longer file names.
Here's what it looks like:
Annotation 2020-05-22 200833

Also added support for the current grid layout to be resized as well as switching between layout modes by using ctrl+scroll or ctrl + +- (#756 point 4)

Here's an example of switching between modes using the ctrl shortcuts:
LayoutModes

When resizing the grid layout using the ctrl shortcuts, icons only refresh their quality/size on a manual refresh. This is just to prevent the performance impacts of reloading the file thumbnails everytime that a user resizes the grid. Thumbnail sizes for the grid view is calculated based on the current grid size to provide a decent quality thumbnail.

One thing that could need improvement would be the layout selection flyout on the bottom right, I've added tiles layout and three general grid sizes (large, medium and small). I had to create icons for the grid sizes, but I'm not quite sure if they fit in, so if anyone wants to redo the icons they are more than welcome to!
Annotation 2020-05-22 195115

@Dylan-Osborne
Copy link
Contributor Author

If anyone's wondering, the TilesBrower is based on the original PhotoAlbum, so it shares a large portion of the backend code. I decided to keep them as seperate controls as the tiles layout is static while the grid layout has quite a few dynamic properties due to the resize functionality.

@ghost ghost added the ready for review Pull requests that are ready for review label May 22, 2020
@tsvietOK
Copy link
Contributor

tsvietOK commented May 22, 2020

@Dylan-Osborne I don't think it is a good idea to duplicate PhotoAlbum class. PhotoAlbum.xaml.cs equals to TilesBrowser.xaml.cs. As i understand difference only in xaml files in Data templates, so why don't to use several data templates instead of different layouts?

@Dylan-Osborne
Copy link
Contributor Author

@tsvietOK Thanks for picking that up, I didn't know you could do that. I'll have a look and see what I can come up with data templates.

@xpoppyx That should be easy enough to do. Currently it's only a once off that you'd need to refresh after resizing the grid, you could also browse to a different directory to achieve the same effect.

@lukeblevins lukeblevins requested review from lukeblevins and yaira2 May 22, 2020 20:11
@tsvietOK
Copy link
Contributor

@Dylan-Osborne examples on how to use different data templates you can find here and here

@Dylan-Osborne
Copy link
Contributor Author

@tsvietOK I've managed to get everything working using data templates. I've had to use a DataTemplateSelector to choose which layout to use, but I've noticed that it executes the selector at least once for each listed item. This is bad for performance, there's even a warning on it's docs

Is there a way to select a data template once off instead of for each listed item?
Here's how I've been using the data template so far xaml and cs

@tsvietOK
Copy link
Contributor

tsvietOK commented May 23, 2020

@Dylan-Osborne As i understand, DataTemplateSelector is used to define different templates(views) to items at the same time. We are don't need this. Instead try to use ItemTemplate property to set one template for all items (look my example above)

@tsvietOK
Copy link
Contributor

@Dylan-Osborne When using dark theme, one icon still black
image

@tsvietOK
Copy link
Contributor

@Dylan-Osborne Not necessarily, but you can add ability to translate new strings by adding them into strings-us file:
StatusBarControlTilesView.Text
StatusBarControlGridViewSmall.Text
StatusBarControlGridViewMedium.Text
StatusBarControlGridViewLarge.Text

@tsvietOK
Copy link
Contributor

@Dylan-Osborne For large icon you can use font icon E71A

@tsvietOK
Copy link
Contributor

I think it is better to add margin at the end of textBlock and textBox
image

@tsvietOK
Copy link
Contributor

And vertically align center name, type and file size

@tsvietOK
Copy link
Contributor

And last what i have found, folder icon is pixelated in GridView large moder
image

@tsvietOK
Copy link
Contributor

For GridView small you can use font icon E80A.
image

@tsvietOK
Copy link
Contributor

@Dylan-Osborne There is a bug. Repro:
Open folder in tiles mode
Change tiles mode to grid view (any size)
Try to open folder context menu
Crash

<Content Include="Assets\FilesHome.png" />
<Content Include="Assets\FolderIcon.svg" />
<Content Include="Assets\GridViewLarge.png" />
<Content Include="Assets\GridViewSmall.png" />
Copy link
Member

Choose a reason for hiding this comment

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

Are the Segoe UI assets you can use instead of these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've used the FontIcon fonts @tsvietOK suggested, it looks much better in the dark theme.

I'll commit the changes as soon as I've fixed the folder bug.

@yaira2
Copy link
Member

yaira2 commented May 24, 2020

@Dylan-Osborne I tested out your changes and I really like them, I noticed two UI issues that should probably be resolved before merging the pull request

  • The UI in tiles view is a little too large.
  • The file name looks a little too high on folders.

image

@Dylan-Osborne
Copy link
Contributor Author

@yaichenbaum Does the tile sizing look a bit better here? I've reduced the width and height of each cell, I don't want to make the height smaller than this as then it won't be able to wrap longer file names to the second line. I've also vertically centered the text details.
image

@lukeblevins
Copy link
Contributor

lukeblevins commented May 24, 2020

@Dylan-Osborne Does the new Tiles layout support lazy loading of extended properties like file type, icon, and size? We currently have it that way in GenericFileBrowser and PhotoAlbum.
Other than that, I'm really impressed with this contribution 🥇

@yaira2
Copy link
Member

yaira2 commented May 24, 2020

@Dylan-Osborne That looks amazing 🙂. Is there anything else we are still waiting on for this pull request to be merged @tsvietOK @duke7553?

@tsvietOK
Copy link
Contributor

@yaichenbaum Wait a bit, as i understand this bug still not fixed

@yaira2
Copy link
Member

yaira2 commented May 24, 2020

@tsvietOK I wasn't able to reproduce that bug on my end.

@Dylan-Osborne
Copy link
Contributor Author

@duke7553 It uses the same backend as the original PhotoAlbum, so it should support lazy loading.
@tsvietOK I've fixed the bug, just haven't committed the changes yet.

@lukeblevins lukeblevins mentioned this pull request May 24, 2020
@yaira2
Copy link
Member

yaira2 commented May 24, 2020

While we are working on this, can you rename PhotoView to GridView?

@tsvietOK
Copy link
Contributor

@yaichenbaum What about GridViewBrowser? Because GridView is a component of Microsoft.UI.Xaml.Controls

@lukeblevins lukeblevins self-requested a review May 24, 2020 18:38
Copy link
Contributor

@lukeblevins lukeblevins left a comment

Choose a reason for hiding this comment

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

Very happy to see this change

@yaira2 yaira2 merged commit 08dfec2 into files-community:master May 24, 2020
@yaira2
Copy link
Member

yaira2 commented May 24, 2020

@Dylan-Osborne Thank you so much for this pull request, this has been merged into master and will roll out with the next release.

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed ready for review Pull requests that are ready for review labels May 25, 2020
@yaira2
Copy link
Member

yaira2 commented May 26, 2020

@xpoppyx Can you please open a separate issue for that?

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