chore: prop setters accessible via ThemeResources #1491
Conversation
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1491.eastus2.azurestaticapps.net |
| <Setter Target="PlaceholderElement.Foreground" Value="{ThemeResource OutlinedTextBoxPlaceholderForegroundFocused}" /> | ||
| <Setter Target="HeaderElement.Foreground" Value="{ThemeResource OutlinedTextBoxHeaderForegroundFocused}" /> | ||
| <Setter Target="RootBorder.BorderThickness" Value="{ThemeResource MaterialOutlinedTextBoxBorderThicknessFocused}" /> | ||
| <Setter Target="RootBorder.Padding" Value="0" /> | ||
| <Setter Target="RootBorder.Padding" Value="{ThemeResource MaterialOutlinedTextBoxPaddingFocused}" /> |
There was a problem hiding this comment.
@kazo0, @eriklimakc, @Xiaoy312 please help me remember if there is a particular reason why we have a mismatch of OutlinedTextBox... and MaterialOutlinedTextBox... for the ThemeResources for example here please
There was a problem hiding this comment.
not that i remember of, other than negligence
There was a problem hiding this comment.
It seems that from the very start of the lightweight style, this was already like that. You can see it here.
There was a problem hiding this comment.
I don't think there is a reason for that, not that I recall
There was a problem hiding this comment.
@kazo0, @Xiaoy312, @eriklimakc We have all these resources below starting with "Material" that we will need to review I think then. I just wanted to make sure I did not forget a specific reason why it was kept "Material" in the name.
And I imagine it will be a breaking change as well for the resource's naming with the documentation update.
- https://github.com/unoplatform/Uno.Themes/blob/master/doc/styles/TextBox.md
Uno.Themes/src/library/Uno.Material/Styles/Controls/v2/TextBox.xaml
Lines 149 to 168 in c35a540
Seems we would need to double-check other controls also (maybe on separate PRs for other controls ? so it is easier to follow the adjustments):
https://github.com/search?q=repo%3Aunoplatform%2FUno.Themes+%22%7C+%60Material%22&type=code
There was a problem hiding this comment.
@Kunal22shah, @kazo0 As mentioned by @Xiaoy312, renaming should be carefully reviewed (and normally a breaking change for lightweight styling, so breaking change should be mentioned in the commit message)
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1491.eastus2.azurestaticapps.net |
| @@ -162,6 +163,9 @@ | |||
| <x:Double x:Key="MaterialOutlinedTextBoxMinHeight">56</x:Double> | |||
| <Thickness x:Key="MaterialOutlinedTextBoxBorderPadding">1</Thickness> | |||
| <x:Double x:Key="MaterialOutlinedTextBoxBorderThicknessFocused">2</x:Double> | |||
| <Thickness x:Key="MaterialOutlinedTextBoxPaddingFocused">0</Thickness> | |||
| <x:Double x:Key="MaterialOutlinedTextBoxBorderThicknessPointerOver">2</x:Double> | |||
| <Thickness x:Key="MaterialOutlinedTextBoxPaddingPointerOver">0</Thickness> | |||
| <!--#endregion--> | |||
| </ResourceDictionary> | |||
|
|
|||
There was a problem hiding this comment.
@Kunal22shah also verify Markup as well just in case please
https://github.com/unoplatform/Uno.Themes/blob/f4256d1e3fee258d24307d2b8d93ba87f64a6633/src/library/Uno.Themes.WinUI.Markup/Theme/TextBox.cs
Co-authored-by: Agnès ZITTE <[email protected]>
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-rock-0cfebe70f-1491.eastus2.azurestaticapps.net |
|
Closing this in favour of #1495 |
GitHub Issue: #1469
PR Type
What kind of change does this PR introduce?
Description
PR Checklist
Please check if your PR fulfills the following requirements:
Other information
Internal Issue (If applicable):