Skip to content
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

Added ability to pin folders to start #4059

Merged
merged 22 commits into from
Mar 21, 2021

Conversation

winston-de
Copy link
Contributor

@winston-de winston-de commented Mar 14, 2021

Closes #782

Tile assets are generated using the Win2D library from the FolderIcon2Large.svg asset and stored in the app cache.
Icons for folders (if there is one) is obtained from GlyphHelper.GetItemIcon. Segoe Fluent Icons are used if they're present on the system, if not Segoe MDL2 Assets are used instead.

image

Menu items for pinning:

Feedback is appreciated :)

- Improved exception handling
- Added check for fluent icons installed, and backup for Segoe MDL2 if they are not
- Remove all symbols and spaces from ID
- Use glyph helper and make glyph param optional
- Moved asset generation to helper class
@yaira2
Copy link
Member

yaira2 commented Mar 14, 2021

@winston-de Looks good! A couple of changes I would suggest

  • Use the regular folders instead of adding the Fluent UI glyphs on top
  • Only show the pin to start menu option when the user opens the menu when pressing down on the shift key

@winston-de
Copy link
Contributor Author

@yaichenbaum May I ask why not the glyphs? In my opinion it makes it much easier to quickly identify items, especially when they're in a folder or are small tiles.
Windows explorer does something similar as well:

image

@yaira2
Copy link
Member

yaira2 commented Mar 14, 2021

@winston-de The glyphs look a bit out of place on the fluent icon, we are also working on some custom icons that might look better here.

Files/Interacts/Interaction.cs Outdated Show resolved Hide resolved
@winston-de
Copy link
Contributor Author

@yaichenbaum Alright, I made the changes requested. I do hope something similar to the glyphs can be implemented in the future, as I think having some sort of icon is really useful :)

Files/Views/LayoutModes/GenericFileBrowser.xaml.cs Outdated Show resolved Hide resolved
Files/Filesystem/ListedItem.cs Outdated Show resolved Hide resolved
Files/Views/LayoutModes/GridViewBrowser.xaml.cs Outdated Show resolved Hide resolved
Files/Files.csproj Outdated Show resolved Hide resolved
Files/Helpers/FolderGlyphAssetHelper.cs Outdated Show resolved Hide resolved
@d2dyno1
Copy link
Member

d2dyno1 commented Mar 18, 2021

@winston-de Some 🔥 ideas for icons:
image

(Source: https://www.youtube.com/watch?v=sLib5QX9qMw&t=50)

@winston-de
Copy link
Contributor Author

winston-de commented Mar 18, 2021

@d2dyno1 That should be made into its own issue as it looks like the icons are not happening this PR.

@R3voA3
Copy link
Contributor

R3voA3 commented Mar 18, 2021

Nice icons, except the one for documents. Isn't there an icon from editor or Wordpad we could use?

@winston-de winston-de requested a review from gave92 March 18, 2021 22:44
@gave92 gave92 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Mar 20, 2021
@yaira2
Copy link
Member

yaira2 commented Mar 21, 2021

@winston-de What assets are you using in the end?

@yaira2 yaira2 merged commit 326ebb2 into files-community:main Mar 21, 2021
@winston-de winston-de deleted the PinFoldersToStart branch March 21, 2021 02:22
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.

5 participants