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 55 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 (elements.GetNumberOfPagesTextBlock().GetText() == "100")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be better to make this a bit more flexible, e.g. try to convert this to int and check if its higher than lets say 50.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or ideally use the magic const 11 we use in the actual control.

{
VerifyNumberBoxEnabled();
VerifyComboBoxDisabled();
VerifyNumberPanelDisabled();
}
else
{
VerifyComboBoxEnabled();
VerifyNumberBoxDisabled();
VerifyNumberPanelDisabled();
}
break;
case DisplayModes.ComboBox:
VerifyComboBoxEnabled();
VerifyNumberBoxDisabled();
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 < 11 ? ComboBoxVisibleVisualState : NumberBoxVisibleVisualState, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

@MarissaMatt The proposal only says something about less than 11 and larger than 11, what about 11 itsself? Is case 11 resulting in NumberBox correct here? I think the spec might need some clarification for this.

Copy link
Author

Choose a reason for hiding this comment

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

@chingucoding can you point me to where it talks about the "less than 11 and larger than 11" on the proposal? I can't seem to find any reference to it there. I know the spec mentions it, but I'm interested to see what the proposal mentions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, it is the first paragraph in this section: https://github.com/microsoft/microsoft-ui-xaml-specs/blob/user/savoyschuler/pagercontrol/active/PagerControl/PagerControl.md#creating-a-pager-control

It states:

"[...] NumberOfPages property is less than 11 and will show the number box mode if it is greater than 11."

Copy link
Contributor

Choose a reason for hiding this comment

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

I updated the spec with that requirement but not the proposal so I will make sure they are both matching. The requirement should be if it's 10 pages or less then combo box will be chosen and if it's 10 or more then number box will be chosen.

Copy link
Author

Choose a reason for hiding this comment

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

I'm still a little confused..

So should it be:
if NumberOfPages <= 10 then use ComboBox else use NumberBox
or
if NumberOfPages < 10 then use ComboBox else use NumberBox

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be if NumberOfPages < 10 then use ComboBox else use NumberBox

Copy link
Author

Choose a reason for hiding this comment

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

Great thanks! I'll update the prototype

break;
case PagerDisplayModes.NumberBox:
VisualStateManager.GoToState(this, NumberBoxVisibleVisualState, false);
break;
case PagerDisplayModes.Auto:
case PagerDisplayModes.ComboBox:
VisualStateManager.GoToState(this, ComboBoxVisibleVisualState, false);
break;
Expand Down
6 changes: 6 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 @@ -85,6 +85,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