Progressring updates #489
Progressring updates #489
Conversation
|
Looks great! One more request - can you add a "New" tag to ProgressRing, just like the AutomationProperties tile has? And probably update ProgressBar's from "Preview" also to "New" while you're in that file. Thank you! |
|
Thank you for making these changes @chingucoding! Double checking - it looks like the black part of the ProgressRing image is fuzzy or something. Is it just me?
|
|
Updated images and controls json file now. |
|
Thanks @chingucoding. I'm still seeing some strange rendering artifacts or something - how are these image files generated? |
|
I've taken a screenshot of the actual progressring, removed background and scaled them down using GIMP The "border" is partially transparent, maybe that is a problem here? |
|
Perhaps. What does it look like in dark theme? Most (all?) of the icons have a solid white border, I believe. So I think we'll need that here as well. |
|
It looks bad:
Unfortunately, I am definitely not an expert on this area :/ |
Are you sure all parts of that Icon change with the Light and Dark theme, it looks like the progress segment is not using white. if you wanted it to all be Opaque, thickness is the only option you can change Or use a dashed line for the track ring |
|
For dark theme however, we would still need a white border of sorts (similiar to other icons) since otherwise, you wouldn't be able to see the icon that well. |
|
For context, many of the other icons are not square nor mono-colored:
|
|
@chingucoding, what about this?
|
|
Those look fine to me. I am a bit concerned how the ring would look like on the NavigationView given that it is being displayed very small, though it would probably be fine. Would you like to update the images or would you like me to? We would first need to remove the background to make it transparent and blend in well with the other icons. |
|
|
|
Thanks @mdtauk - one further clarification: the light and dark theme use the same assets today, meaning we need a single asset that looks good in both themes.
Oh, you're right. Oops. The NavigationView only uses FontIcons right now to avoid an issue where using custom assets wouldn't render on first launch. I think we should avoid using a custom asset in the NavigationView and change the icon to a different font icon.
The first picture I included has a transparent background. |
You could separate the Progress Segment from the Ring, and overlay one Glyph on top of the other - so a Font Icon can still be used. This is what is done for the ink toolbar controls |
Ohhhhh, that makes sense. In that case, the question would be, what the new icon would be if we choose to change it. The current item seems like one of the only ones suitable for this use category. We could use |
But are these distinct parts available as font icons? In addition to that, I am not 100% sure if we would be able to load them from the JSON or if we would need to adjust the current "icon" loading process. |
I just drew those icons ideas in figma - does the gallery just use Segoe MDL2, or a custom font? |
In it's current form, we only use the Segoe MDL2 Assets font. |
That is gonna become problematic as the pace of updates outstrips that of the font. Not to mention the new Fluent Icons only available as SVGs for now |
Agreed, but this is a separate issue from the one that @chingucoding is trying to fix 😅 |
|
Yes, another thing is that we only need a small icon now as the progressring icon is used for one of the categories.
|
Sure. It would also be good to synchronise the icons used in the controls gallery, with the icons used in Visual Studio for the SDK - and so more flexibility with icon formatting (multiple colours) would be ideal |
|
@chingucoding - what about U+E8F2 for the NavigationView icon?
|
|
@chingucoding - is ProgressIcon used anymore? If not, can it be removed? |
I can't find any "ProgressIcon" in the XCG code 😅 Or am I missing something here? |
|
It's one of the files included in the changelist. https://github.com/microsoft/Xaml-Controls-Gallery/pull/489/files#diff-478ec5ba36eaedd60dc7b04727c64127 |
|
Ohhh right, yes that's not needed anymore. Removed it. Totally forgot that. |
There was a problem hiding this comment.
I had this image leftover from a previous PR that you did @chingucoding
|
Haha love it, thank you @stmoy ! |
|
I made a somewhat related issue post some time ago in the WinUI repo, but when I am done with the initial set of icons, I guess I can make a new issue here for it. microsoft/microsoft-ui-xaml#1204
|
|
Those new items look awesome! Would you like me to create a tracking issue in this repo to update them once you are finished and happy with the new icons? |
Hold off until I have a complete set - I may have to do simplified 16px versions as the scaling just does not play nice - but the 72, 48, 24 should work find with these vector shapes. |
|
Ok so I think this is everything included in the Gallery's assets. I am posting it here - each icon at 72x72 and 16x16 size - and they should work on Light and Dark backgrounds. Any which stand out as odd or wrong, or any feedback in particular would be welcome, before making a separate proposal.
|
|
Can you add labels for the icons, that is what icon would correspond to which control? Also I am not sure if the color palette would be the best image for RadioalGradientBrush. Another question, was there a specific reason for going more dark themed icons? Beside that, those new icons look really good! |
I was trying to find an approach that worked equally well for Light and Dark themes - these read well on both to me, but all of it is changeable of course. Here is the Figma file with the icons in. They are in Frames with names on. I used the Treeview from the Gallery app to sort them. The RadialGradientBrush is in the motion section for whatever reason - I think it should probably go into Styles however. Shared WinUI Gallery Icons.zip EDIT: Here are the colours inverted quickly in Photoshop - if that seems a better approach? |
|
Oh yes, RadialGradientBrush is in the style section now, but not in the store version yet I think. |
Here are the icons with the colours inverted. EDIT: Inverted version of the Figma added |
|
Those look awesome! Thank you for helping and making those. @stmoy What do you think? |
|
I fixed the Gradient Icon, and added the Figma with the inverted colours |
|
Labels Added I will say I am a little concerned that the icons - now inverted - may appear very stark in the Dark Theme - where as the un-inverted versions, wont appear too harsh with the Light Theme... |
|
Thank you for the updated version with labels. Regarding your dark theme concern, this is how it looks like with a dark background:
|
At a first glance it appears fine, but after long periods? I know this is part of the reason colours are reduced with Dark Mode UI. I could be overthinking it perhaps? |
That's a good point. I don't think that you are overthinking this, it is definitely a valid concern. Currently the icons have around the same brightness:
I suppose it was fine back then, so it is now? I never felt that the icons are too bright over time, especially since you actually don't spend too much time with them. Generally you search for a page and visit said page, and are looking to much at those control "overviews". |
|
Eitherway, I think the icons work both inverted and un-inverted, so they are out there now if the community and the Repo owners want them. It would be nice if the Visual Studio toolbox synced with a single set of icons, but that is a much bigger ask 😛 The figma file is there too if tweaks are needed, so I will await comments. |
|
Thanks for including this, @mdtauk! I think we'd want to update both the WinUI icons and the Xaml Controls Gallery icons at the same time. CC: @ranjeshj - thoughts on cost to do this? I think the next steps would be to file one more issue on the XCG repo saying "update the icons alongside microsoft/microsoft-ui-xaml#1204", and include a link to all of the image assets. I think two issues are appropriate and they should probably be done at around the same time. CC: @chigy for visibility as well |
|
@stmoy Do you need me to make an issue for this, or is that something you will be doing? |
|
Filed: #504 |
* Update image, fix issues with progressring page * Update images * Update images * Update new status of progress controls * Switch icon in NavView * Add updated image * Remove unused icon
Description
Update images, the namespaces used in the sample code and the namespace used in the page code.
Motivation and Context
Closes #485
How Has This Been Tested?
Tested manually
Screenshots (if appropriate):
Types of changes