Skip to content

Remove EquationTextBox from KeyGraphFeaturesPanel #833

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
Merged
merged 4 commits into from
Dec 17, 2019
Merged

Remove EquationTextBox from KeyGraphFeaturesPanel #833

merged 4 commits into from
Dec 17, 2019

Conversation

@rudyhuyn
Copy link
Contributor

@rudyhuyn rudyhuyn commented Nov 28, 2019 •
edited
Loading

Description of the changes:

  • Replace the templatized EquationTextBox in KeyGraphFeaturesPanel with a MathTextBox

Also:

  • Give focus to the back button when the user opens the panel
  • move styles only used by Graph Calculator from App.xaml.cs to EquationInputArea/GraphingCalculator to improve startup time
  • NIT: make the names of events used by the EquationInputArea control uniform.
  • simplify KeyGraphFeaturesPanel
  • simplify the XAML code of the toggle button used in EquationTextBox

How changes were validated:

  • manually

@rudyhuyn rudyhuyn requested a review from sanderl November 28, 2019 02:19
@rudyhuyn rudyhuyn added the codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) label Nov 28, 2019
@sanderl sanderl added the graphing calculator Work items related to the graphing calculator feature. label Dec 2, 2019
@rudyhuyn
Copy link
Contributor Author

rudyhuyn commented Dec 4, 2019

PR updated!

joseartrivera
joseartrivera requested changes Dec 4, 2019
View reviewed changes
Copy link
Contributor

@joseartrivera joseartrivera left a comment

Choose a reason for hiding this comment

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

Thanks for the code clean up, this is looking good, left a few comments.

joseartrivera
joseartrivera reviewed Dec 5, 2019
View reviewed changes
EriWong
EriWong previously approved these changes Dec 16, 2019
View reviewed changes
Copy link
Contributor

@EriWong EriWong left a comment

Choose a reason for hiding this comment

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

Looked through the code and played around with the dark mode changes. I think we still have a bug with black on black text in one instance, but that is present even without this change and can be addressed separately. Thanks for doing this!

@rudyhuyn
Copy link
Contributor Author

We can't merge this PR without Pepe's approval. In the meantime, I will include the diff to fix the black text on black background in this PR.

@grochocki grochocki merged commit dc79ec6 into microsoft:feature/GraphingCalculator Dec 17, 2019
@rudyhuyn rudyhuyn deleted the ReplaceEquationTextBoxInKeyFeatures branch December 17, 2019 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@grochocki grochocki grochocki approved these changes

@sanderl sanderl Awaiting requested review from sanderl

@joseartrivera joseartrivera Awaiting requested review from joseartrivera

+1 more reviewer

@EriWong EriWong EriWong left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@EriWong EriWong

Labels

codebase quality Issues that are not bugs, but still might be worth improving (eg, code hygiene or maintainability) graphing calculator Work items related to the graphing calculator feature.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rudyhuyn @grochocki @EriWong @joseartrivera @sanderl