Skip to content

Update Calculator keyboard for greater consistency and extensibility#688

Merged
joseartrivera merged 10 commits intomicrosoft:masterfrom
joseartrivera:joriv/keyboardrefresh
Oct 3, 2019
Merged

Update Calculator keyboard for greater consistency and extensibility#688
joseartrivera merged 10 commits intomicrosoft:masterfrom
joseartrivera:joriv/keyboardrefresh

Conversation

@joseartrivera
Copy link
Copy Markdown
Contributor

@joseartrivera joseartrivera commented Sep 30, 2019

Fixes #428

Spec for this change can be found here:
Update Calculator keyboard for greater consistency and extensibility

Description of the changes:

This PR includes the UI changes for the keyboard refresh spec.

  • Added corresponding UI buttons for new calc functions added in Update Calc Engine for new functions needed for keyboard refresh #662
  • Rearranged some of the buttons in the Standard, Scientific, and Programmer modes
  • Added OperatorPanelButton that displays a flyout when clicked, storing buttons that are closely related
  • C and CE button are conditionally visible based on if there is input that can be cleared
  • Calc Buttons can be navigated with arrow keys
  • Visual State layouts tweaked and simplified per the spec linked above
  • Enable negate buttons in some conversion modes per the spec linked above

How changes were validated:

  • Manual testing

joseartrivera
joseartrivera commented Sep 30, 2019
View reviewed changes
sanderl
sanderl reviewed Oct 1, 2019
View reviewed changes
sanderl
sanderl reviewed Oct 1, 2019
View reviewed changes
sanderl
sanderl reviewed Oct 1, 2019
View reviewed changes
sanderl
sanderl reviewed Oct 1, 2019
View reviewed changes
@sanderl
Copy link
Copy Markdown
Contributor

sanderl commented Oct 1, 2019 

                  FontSize="20"

Should the font size always be 20?

Refers to: src/Calculator/Views/CalculatorScientificOperators.xaml:878 in 8c260fa. [](commit_id = 8c260fa, deletion_comment = False)

@sanderl
Copy link
Copy Markdown
Contributor

sanderl commented Oct 1, 2019 

                               Content="e"/>

Should the font sizes be constant here too?

Refers to: src/Calculator/Views/CalculatorScientificOperators.xaml:903 in 8c260fa. [](commit_id = 8c260fa, deletion_comment = False)

@sanderl
Copy link
Copy Markdown
Contributor

sanderl commented Oct 1, 2019 

                                   Content=""/>

constant font sizes

Refers to: src/Calculator/Views/CalculatorScientificOperators.xaml:943 in 8c260fa. [](commit_id = 8c260fa, deletion_comment = False)

@sanderl
Copy link
Copy Markdown
Contributor

sanderl commented Oct 1, 2019 

                               FontSize="19"

constant font size for this button and the next 2 buttons as well

Refers to: src/Calculator/Views/CalculatorScientificOperators.xaml:1122 in 8c260fa. [](commit_id = 8c260fa, deletion_comment = False)

@sanderl sanderl self-requested a review October 1, 2019 20:55
sanderl
sanderl reviewed Oct 1, 2019
View reviewed changes
sanderl
sanderl reviewed Oct 1, 2019
View reviewed changes
sanderl
sanderl reviewed Oct 1, 2019
View reviewed changes
sanderl
sanderl suggested changes Oct 1, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@sanderl sanderl left a comment

Choose a reason for hiding this comment

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

Some minor things that i caught.

@ghost ghost added the needs author feedback label Oct 1, 2019
@ghost ghost removed the needs author feedback label Oct 1, 2019
@joseartrivera joseartrivera merged commit 55074c2 into microsoft:master Oct 3, 2019
@rudyhuyn
Copy link
Copy Markdown
Contributor

rudyhuyn commented Oct 11, 2019

I'm a little late but congratulations on this PR, it looks a lot better! 🎉🍾

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DavidShoe DavidShoe Awaiting requested review from DavidShoe

1 more reviewer

@sanderl sanderl sanderl approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

fixing approved issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update Calculator keyboard for greater consistency and extensibility

4 participants

@joseartrivera @sanderl @rudyhuyn @grochocki