Tabview switch to converter instead of template settings #2827
Conversation
msft-github-bot
added
the
needs-triage
| [WUXC_VERSION_PREVIEW] | ||
| [webhosthidden] | ||
| [default_interface] | ||
| runtimeclass CornerRadiusToFilteredMultipliedThicknessConverter : Windows.UI.Xaml.DependencyObject, Windows.UI.Xaml.Data.IValueConverter |
There was a problem hiding this comment.
Could we use CornerRadiusToThicknessConverter and add the Multiplier property?
There was a problem hiding this comment.
Unfortunately in it's current form, the CornerRadiusToThicknessConverter wouldn't help us with a multiplier property. The CornerRadiusToThicknessConverter always sets two directions to zero and the other two to the corner radius value.
Essentially it would move the item diagonally. In our case however, we would need to move the item the left/right which the CornerRadiusToThicknessConverter can't provide.
There was a problem hiding this comment.
The existing CornerRadiusToThicknessConverter sets two components from the Thickness from the CornerRadius, and sets the rest to zero. The new converter needs to take one component from the CornerRadius and set the rest to zero. So couldn't we reuse it with a new CornerRadiusToThicknessConverterKind (plus the multipler)?
public enum CornerRadiusToThicknessConverterKind
{
FilterTopAndBottomFromLeft = 0,
FilterTopAndBottomFromRight = 1,
FilterLeftAndRightFromTop = 2,
FilterLeftAndRightFromBottom = 3,
// new
FilterLeftFromBottomLeft
FilterTopFromTopLeft
...
}
There was a problem hiding this comment.
It seems like you could expand the CornerRadiusToThicknessConverterKind to have more enum values like FilterTopFromTopLeft and then also add the multiplier property
In reply to: 450386895 [](ancestors = 450386895)
There was a problem hiding this comment.
All right, I'll update the existing converter. So we would introduce 8 new values to the enum right?
LeftFromBottomLeft,
LeftFromTopLeft,
TopFromTopLeft,
TopFromTopRight,
and so on.
There was a problem hiding this comment.
Updated the existing converter.
| break; | ||
| case winrt::CornerRadiusToThicknessSide::Top: | ||
| result.Left = 0; | ||
| result.Top = radius.TopLeft * multiplier; |
There was a problem hiding this comment.
radius.TopLeft * multiplie [](start = 21, length = 26)
How did you decide that this should come from radius.TopLeft instead of radius.TopRight?
There was a problem hiding this comment.
I "rotated" the corner radii by 45° clockwise. That way, bottomleft becomes left, topleft becomes top etc. The 45° clockwise were chosen arbitrarily.
There was a problem hiding this comment.
Should we do something like, for result.Top, take the max of TopLeft and TopRight?
There was a problem hiding this comment.
Given that this converter is removed now, should we add this behavior to the CornerRadiusToThicknessConverter? With the current state of the PR, developers would be able to choose whether the left thickness should come from TopLeft or BottomLeft of the corner radius.
|
Please merge from master to pick up the GSL const enforcement change and rebuild to make sure you aren't missing something that will block on. |
Good point. |
StephenLPeters
added
area-TabView
team-Controls
|
Added an API spec: microsoft/microsoft-ui-xaml-specs#90 |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Looks like last test run failed due to heap limit. Merged with master to get latest fix for that. |
one more time :) |
|
I think my latest merge with master already has the newest fix. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@teaP Feel free to merge this in if you don't find any changes needed |
Description
Switched to unfortunately quite specific converters to remove template settings.
The naming of the new converter and its properties can definitely be changed!
Motivation and Context
Remove template settings to keep view and logic separated. See this thread for more context.
How Has This Been Tested?
Run app and verify visuals (TabView looks the same).
Screenshots (if appropriate):