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

Update InfoBarPanel APIs to match existing concepts of padding and spacing #3701

Closed
ranjeshj opened this issue Nov 25, 2020 · 2 comments · Fixed by #3704
Closed

Update InfoBarPanel APIs to match existing concepts of padding and spacing #3701

ranjeshj opened this issue Nov 25, 2020 · 2 comments · Fixed by #3704
Assignees
Labels
area-InfoBar help wanted Issue ideal for external contributors team-Controls Issue for the Controls team
Milestone

Comments

@ranjeshj
Copy link
Contributor

ranjeshj commented Nov 25, 2020

Update the naming of properties on InfoBarPanel to match existing concepts of padding and spacing.

Rename HorizontalMargin and VerticalMargin attached properties and use them just for the children. Currently we are also setting these on InfoBarPanel itself. Instead we should be using the new padding properties named as below for that purpose.

unsealed runtimeclass InfoBarPanel : Windows.UI.Xaml.Controls.Panel
{
    InfoBarPanel();

   // Normal dependency properties to set padding around the children 
    Windows.UI.Xaml.Thickness PaddingInHorizontalOrientation; 
    static Windows.UI.Xaml.DependencyProperty PaddingInHorizontalOrientationProperty{ get; };}

   Windows.UI.Xaml.Thickness PaddingInVerticalOrientation;
    static Windows.UI.Xaml.DependencyProperty PaddingInVerticalOrientationProperty{ get; };}

    // Attached properties to define spacing for each child 
    static void SetSpacingInHorizontalOrientation (Windows.UI.Xaml.DependencyObject object, Windows.UI.Xaml.Thickness value);
    static Windows.UI.Xaml.Thickness GetSpacingInHorizontalOrientation (Windows.UI.Xaml.DependencyObject object);
    static Windows.UI.Xaml.DependencyProperty SpacingInHorizontalOrientationProperty{ get; };
   
    static void SetSpacingInVerticalOrientation (Windows.UI.Xaml.DependencyObject object, Windows.UI.Xaml.Thickness value);
    static Windows.UI.Xaml.Thickness GetSpacingInVerticalOrientation (Windows.UI.Xaml.DependencyObject object);
    static Windows.UI.Xaml.DependencyProperty SpacingInVerticalOrientationProperty{ get; };
}

These will show up in markup like so (ignore values).

<InfoBarPanel PadingInHorizontalOrientation=”x,x,x,x” PaddingInVerticalOrientation=”x,x,x,x”>
       <ChildControl   InfoBarPanel.SpacingInHorizontalOrientation=”x,x,x,x” InfoBarPanel.SpacingInVerticalOrientation=”x,x,x,x” />
        …
</>

And the theme resources follow this pattern
• InfoBar(Title|Message|Action)(Horizontal|Vertical)LayoutMargin  InfoBar(Title|Message|Action)SpacingIn(Horizontal|Vertical)Orientation
• InfoBarPanel(Horizontal|Vertical)LayoutMargin  InfoBarPanelPaddingIn(Horizontal|Vertical)Orientation

@ranjeshj ranjeshj added help wanted Issue ideal for external contributors team-Controls Issue for the Controls team area-InfoBar labels Nov 25, 2020
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Nov 25, 2020
@ranjeshj ranjeshj removed the needs-triage Issue needs to be triaged by the area owners label Nov 25, 2020
@ranjeshj ranjeshj added this to the WinUI 2.5 milestone Nov 25, 2020
@marcelwgn
Copy link
Contributor

I would like to work on this.

@robloo
Copy link
Contributor

robloo commented Nov 29, 2020

I'm never going to be able to talk you out of what was decided in API review; however, something along the lines of below makes more sense to me. I wouldn't expect to see "In" used within property names. Historically, words within a property or class name were just reordered to remove connecting words like this.

Thickness HorizontalOrientationPadding; 
DependencyProperty HorizontalOrientationPaddingProperty{ get; };}

Thickness VerticalOrientationPadding;
DependencyProperty VerticalOrientationPaddingProperty{ get; };}

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Nov 29, 2020
@StephenLPeters StephenLPeters removed the needs-triage Issue needs to be triaged by the area owners label Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-InfoBar help wanted Issue ideal for external contributors team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants