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.

Sign up for GitHub

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

Jump to bottom

Pager: Add Auto Display Mode functionality #3239

Merged
merged 57 commits into from
Sep 4, 2020
Merged

Pager: Add Auto Display Mode functionality #3239

Show file tree
Hide file tree
Changes from 56 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
089f8c0
Initial setup for Pager
Jul 7, 2020
88cf084
Initial Build for MUXControlsInnerLoop
Jul 7, 2020
aaac224
Added Pager Prototype for NumberBox & ComboBox
Jul 14, 2020
6e45803
Added Pager Interaction Tests for Edge Buttons and Display Modes.
Jul 22, 2020
6cb0cf4
Delete unnecessary file
Jul 22, 2020
27b8f07
Updated InnerLoop files
Jul 22, 2020
6dbb145
Removed unnecessary statements, added comment.
Jul 23, 2020
3742723
Fixed Conflicts with master
Jul 23, 2020
44908d8
Added Test Suites to Pager Tests
Jul 30, 2020
0118bb3
Refactored tests, removed redundant tests, removed C++ Pager and var…
Jul 31, 2020
1561e7d
Undo small change from last commit and added better names to xaml
Aug 1, 2020
d1d95f0
Merge branch 'master' into master
Aug 3, 2020
2c8c8a6
Removed some converters
Aug 3, 2020
e01dd1a
Merge branch 'master' of https://github.com/gvazquez8/microsoft-ui-xaml
Aug 3, 2020
b31a814
Added button width resource for NumberPanel mode
Aug 3, 2020
0c5a85b
Updated visual state variable names in codebehind and updated font fa…
Aug 3, 2020
2e8cfdc
Made property changed events non static and attached to properties wi…
Aug 3, 2020
40828c1
Removed unnecessary setters in visual states
Aug 3, 2020
f562f14
Delete Pager.properties.cpp
Aug 3, 2020
5aebd68
Delete Pager.properties.h
Aug 3, 2020
8216c1b
Delete Pager.cpp
Aug 3, 2020
b210a0d
Delete Pager.h
Aug 3, 2020
fadef53
Delete Pager.idl
Aug 3, 2020
35ee782
Delete Pager.vcxitems
Aug 3, 2020
9665cea
Delete Pager.xaml
Aug 3, 2020
089c575
Added PreviousPage parameter to PageChangedEventArgs. Adjusted TestPa…
Aug 3, 2020
a711afa
Merge branch 'master' of https://github.com/gvazquez8/microsoft-ui-xaml
Aug 3, 2020
ac2e16a
Revert "Delete Pager.vcxitems"
Aug 4, 2020
ed59b07
Revert "Delete Pager.xaml"
Aug 4, 2020
11cebd3
Revert "Delete Pager.idl"
Aug 4, 2020
a776308
Revert "Delete Pager.h"
Aug 4, 2020
05a8bc7
Revert "Delete Pager.cpp"
Aug 4, 2020
17cc715
Revert "Delete Pager.properties.h"
Aug 4, 2020
efe037d
Revert "Delete Pager.properties.cpp"
Aug 4, 2020
e13f307
Resolved build issues
Aug 4, 2020
e5817bc
Removed unnecessary line in App.xaml.cs
Aug 4, 2020
e1ac9ac
Delete generated files for pager c++ proj
Aug 4, 2020
e356d12
Removed duplicate function
Aug 4, 2020
f350481
Changed access modifier for TemplateSettings. (public -> internal). M…
Aug 4, 2020
2a04aa2
Removed setter for TemplateSettings. Modified name of TemplateSetting…
Aug 4, 2020
9700d2d
Undo changes in Microsoft.UI.Xml.vcxproj.filters
Aug 4, 2020
92b3776
Made template settings property private.
Aug 4, 2020
304347f
Refactored Test Code even more
Aug 4, 2020
25aa4e2
Added new visual states for IsEnabled property on buttons and refacto…
Aug 5, 2020
e247b68
Removed MinusOneConverter and adjusted code to not rely on it
Aug 5, 2020
887e445
Made some string constants and removed converter reference
Aug 5, 2020
a20af66
Removed unnecessary variable
Aug 5, 2020
82d8115
Gave visual state better names in code behind
Aug 6, 2020
c6c2911
Merge https://github.com/microsoft/microsoft-ui-xaml
Aug 13, 2020
6a053e3
Merge https://github.com/microsoft/microsoft-ui-xaml
Aug 24, 2020
cf87ea4
Merge https://github.com/microsoft/microsoft-ui-xaml
Aug 26, 2020
059b0f4
Added Auto mode behavior.
Aug 26, 2020
924e980
Merge https://github.com/microsoft/microsoft-ui-xaml
Sep 3, 2020
8121b28
Merge in master
Sep 3, 2020
409172c
Modified auto display mode tests
Sep 3, 2020
4c3312a
Changed threshold value for auto display mode and removed magic numbers.
Sep 3, 2020
b655165
Added test to check auto display mode threshold boundry
Sep 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion dev/Pager/InteractionTests/PagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,17 @@ public void ChangingDisplayModeTest()

SetNumberPanelDisplayMode();
VerifyNumberPanelDisplayMode();

ChangeNumberOfPages();
VerifyNumberOfPages("100");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100 [](start = 37, length = 3)

I would also verify the boundaries, since the display mode changes at 10 we should be testing 9 and 10 as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


SetAutoDisplayMode();
VerifyAutoDisplayMode();

ChangeNumberOfPages();
VerifyNumberOfPages("5");

VerifyAutoDisplayMode();
}
}

Expand Down Expand Up @@ -580,7 +591,7 @@ int GetPreviousPageAsInt()
{
return Convert.ToInt32(GetPreviousPage());
}
string GetPreviousPage()
string GetPreviousPage()
{
return elements.GetPreviousPageTextBlock().GetText();
}
Expand Down Expand Up @@ -812,6 +823,19 @@ void VerifyDisplayMode(DisplayModes mode)
switch (mode)
{
case DisplayModes.Auto:
if (Convert.ToInt32(elements.GetNumberOfPagesTextBlock().GetText()) < Convert.ToInt32(elements.GetAutoDisplayModeThresholdValueTextBlock().GetText()))
{
VerifyComboBoxEnabled();
VerifyNumberBoxDisabled();
VerifyNumberPanelDisabled();
}
else
{
VerifyNumberBoxEnabled();
VerifyComboBoxDisabled();
VerifyNumberPanelDisabled();
}
break;
case DisplayModes.ComboBox:
VerifyComboBoxEnabled();
VerifyNumberBoxDisabled();
Expand Down
6 changes: 6 additions & 0 deletions dev/Pager/InteractionTests/PagerTestsPageElements.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,12 @@ public TextBlock GetPreviousPageTextBlock()
}
private TextBlock PreviousPageTextBlock;

public TextBlock GetAutoDisplayModeThresholdValueTextBlock()
{
return GetElement(ref AutoDisplayModeThresholdValueTextBlock, "AutoDisplayModeThresholdValueTextBlock");
}
private TextBlock AutoDisplayModeThresholdValueTextBlock;

private T GetElement<T>(ref T element, string elementName) where T : UIObject
{
if (element == null)
Expand Down
5 changes: 4 additions & 1 deletion dev/Pager/PrototypePager/PrototypePager.Events.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ private void OnNumberOfPagesChanged()
}

DisablePageButtonsOnEdge();
OnPagerDisplayModeChanged();
}

private void OnComboBoxSelectionChanged()
Expand All @@ -113,10 +114,12 @@ private void OnPagerDisplayModeChanged()
{
switch (this.PagerDisplayMode)
{
case PagerDisplayModes.Auto:
VisualStateManager.GoToState(this, NumberOfPages < AutoDisplayModeNumberOfPagesThreshold ? ComboBoxVisibleVisualState : NumberBoxVisibleVisualState, false);
break;
case PagerDisplayModes.NumberBox:
VisualStateManager.GoToState(this, NumberBoxVisibleVisualState, false);
break;
case PagerDisplayModes.Auto:
case PagerDisplayModes.ComboBox:
VisualStateManager.GoToState(this, ComboBoxVisibleVisualState, false);
break;
Expand Down
2 changes: 2 additions & 0 deletions dev/Pager/PrototypePager/PrototypePager.TestHooks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ public sealed partial class PrototypePager : Control
public ComboBox ComboBoxDisplayTestHook;
public ItemsRepeater NumberPanelDisplayTestHook;
public Rectangle NumberPanelCurrentPageIdentifierTestHook;

public int AutoDisplayModeNumberOfPagesThresholdTestHook;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoDisplayModeNumberOfPagesThresholdTestHook [](start = 19, length = 45)

It looks like your test app doesn't use this expect to show the value. In a production environment these are not free and I think we should avoid them if they aren't useful. For the production version of this I would just hard code the value in the test app.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed it!

}
}
4 changes: 3 additions & 1 deletion dev/Pager/PrototypePager/PrototypePager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ public enum ButtonVisibilityMode { Auto, AlwaysVisible, HiddenOnEdge, None, }
private ItemsRepeater PagerNumberPanel;
private Rectangle NumberPanelCurrentPageIdentifier;

private static int AutoDisplayModeNumberOfPagesThreshold = 10; // This integer determines when to switch between NumberBoxDisplayMode and ComboBoxDisplayMode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just putting this out as a thought: Should we make this customizable? Of course not in this PR, but maybe this is something we could think about for the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this adds very much, if you want to customize this you could just change your display mode from combo box to numberbox at your chosen threshold, which doesn't seem like an issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, though with that argument you could also argue why we need this in the first place given that devs can implement it themselves. But yeah, it probably doesn't add much to the API.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this gets us the default behavior that our designers think is best. If another app's designer wants a different behavior they can change that but if they want to default to our design this does that for them

In reply to: 483778517 [](ancestors = 483778517)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, that's a very good point. That ensures that app's all use the recommended behavior if they aren't actively trying to work around that. This shouldn't be customizable then.

private static int MaxNumberOfElementsInRepeater = 7;
private static int NumberPanelMiddleStateStartIndex = 5;
private int NumberPanelEndStateStartIndex;

private static IconElement LeftEllipse = new SymbolIcon(Symbol.More);
private static IconElement RightEllipse = new SymbolIcon(Symbol.More);

Expand Down Expand Up @@ -111,6 +112,7 @@ protected override void OnApplyTemplate()
ComboBoxDisplayTestHook = PagerComboBox;
NumberPanelDisplayTestHook = PagerNumberPanel;
NumberPanelCurrentPageIdentifierTestHook = NumberPanelCurrentPageIdentifier;
AutoDisplayModeNumberOfPagesThresholdTestHook = AutoDisplayModeNumberOfPagesThreshold;

// Attach click events
if (FirstPageButton != null)
Expand Down
4 changes: 4 additions & 0 deletions dev/Pager/TestUI/PagerPage.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@
<TextBlock Text="Current Page: "/>
<TextBlock x:Name="CurrentPageTextBlock" AutomationProperties.Name="CurrentPageTextBlock"/>
</StackPanel>
<StackPanel Orientation="Horizontal">
<TextBlock Text="AutoDisplayMode Threshold Value: "/>
<TextBlock x:Name="AutoDisplayModeThresholdValueTextBlock" AutomationProperties.Name="AutoDisplayModeThresholdValueTextBlock"/>
</StackPanel>
</StackPanel>

<StackPanel Grid.Row="5" Grid.Column="0">
Expand Down
8 changes: 8 additions & 0 deletions dev/Pager/TestUI/PagerPage.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ private void OnLoad(object sender, RoutedEventArgs args)
NumberBoxIsEnabledCheckBox.IsChecked = TestPager.NumberBoxDisplayTestHook.IsEnabled;
ComboBoxIsEnabledCheckBox.IsChecked = TestPager.ComboBoxDisplayTestHook.IsEnabled;
UpdateNumberPanelContentTextBlock(this, null);

AutoDisplayModeThresholdValueTextBlock.Text = TestPager.AutoDisplayModeNumberOfPagesThresholdTestHook.ToString();
}

private void OnElementPrepared(ItemsRepeater sender, ItemsRepeaterElementPreparedEventArgs args)
Expand Down Expand Up @@ -85,6 +87,12 @@ private void NumberOfPagesSetterButtonClicked(object sender, RoutedEventArgs arg
TestPager.NumberOfPages = 5;
NumberOfPagesSetterButton.Content = "Set NumberOfPages to 100";
}

NumberBoxVisibilityCheckBox.IsChecked = TestPager.NumberBoxDisplayTestHook.Visibility == Visibility.Visible;
ComboBoxVisibilityCheckBox.IsChecked = TestPager.ComboBoxDisplayTestHook.Visibility == Visibility.Visible;
NumberPanelVisibilityCheckBox.IsChecked = TestPager.NumberPanelDisplayTestHook.Visibility == Visibility.Visible;
NumberBoxIsEnabledCheckBox.IsChecked = TestPager.NumberBoxDisplayTestHook.IsEnabled;
ComboBoxIsEnabledCheckBox.IsChecked = TestPager.ComboBoxDisplayTestHook.IsEnabled;
}

private void OnPageChanged(PrototypePager sender, PageChangedEventArgs args)
Expand Down