Open Jay-o-Way opened 3 months ago
@jay-o-way Something you'd like to pick up?
@niels9001 I think these lines are unused and can be removed, which means the two top Grids can be removed. Can you confirm? https://github.com/microsoft/WinUI-Gallery/blob/6be8d2d8c1d647f0bacee437ba77326450ef5e39/WinUIGallery/Controls/DesignGuidance/ColorPageExample.xaml#L15 https://github.com/microsoft/WinUI-Gallery/blob/6be8d2d8c1d647f0bacee437ba77326450ef5e39/WinUIGallery/Controls/DesignGuidance/ColorTile.xaml#L17
@niels9001 I think these lines are unused and can be removed, which means the two top Grids can be removed. Can you confirm?
Yes, even the root grid can be removed here?
I think we are using that columndefinition for the border?
I think we are using that columndefinition for the border?
Ah yes, the ShowSeparator. Thing is - in practice it isn't used, so I already removed it (locally) when working on this. Therefore the ColumnDefinition is obsolete.
I think we are using that columndefinition for the border?
Ah yes, the ShowSeparator. Thing is - in practice it isn't used, so I already removed it (locally) when working on this. Therefore the ColumnDefinition is obsolete.
Just looking at the code.. it's turned on by default:
And in 8 instances we turn it off https://github.com/search?q=repo%3Amicrosoft%2FWinUI-Gallery%20ShowSeparator&type=code?
So I think we are using it?
@niels9001 No, it is not used. Well, technically in two occasions, but it's invisible! Also, the search result counts files, not instances.
@jay-o-way It is used in some occasions, and it is visible for me?
Set to true:
Set to false:
I think, since we are use it here and there to make the default be false
and only turn it on when needed to reduce the verbosity of XAML. But other than that, I think we need to keep it..
I see. Focussed on the declared ("false") values and missed the instances where it wasn't declared. 🫣 We could elevate this topic to a higher level: why not choose to use it either always or never?
Ugh, my brain keeps thinking more and more things. Maybe, one day, we can combine all the issues with these Color things, and refactor the whole thing..? Using a very large UserControl with bound data is a big work, but might solve a couple of issues.
I see. Focussed on the declared ("false") values and missed the instances where it wasn't declared. 🫣 We could elevate this topic to a higher level: why not choose to use it either always or never?
This was per design spec. We only need it when 2 neighboring colors are similar to provide the right contrast. I don't think there's a problem here that needs to be solves? Having it default to false
would be my only proposed change.
On the CornerRadius issues - yes, we should remove any hardcoded 4px radiuses if those are defined. Not sure why those were defined in the first place though 😨. (Could be that the grid style was added later, and we missed removing a few of these).
I've been digging into this now, but the corners are weird. Looks like it's not a matter of a property, but more of the rendering...
Which version of the app?
WinUI 3 Gallery
Description
Minor issue.
CornerRadius="4"
is used. Not only is it hard-coded, it's also the incorrect value for these cases.Screenshots