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

Adding support for handling SpellCheck.IsEnabled on TextBoxes. #650

Merged
merged 2 commits into from
Apr 19, 2017

Conversation

Keboo
Copy link
Member

@Keboo Keboo commented Mar 8, 2017

Fixes #497

Though I understand that the intent it for this library to be a theme library, not a control library, it seems like a loss in functionality for someone moving from standard WPF.
The intent of this was to simply handle the cases where someone merely sets SpellCheck.IsEnabled="True" and then includes this theme library.

I have attempted to handle all possible cases, with a few things in particular worth pointing out.

  • The feature was implemented with an attached property in TextFieldAssist.IncludeSpellingSuggestions, and bound its default value to SpellCheck.IsEnabled. This should allow for people wanting to roll their own UI for the spell check (and thus have SpellCheck.IsEnabled="True"), and simply want to opt-out of the behavior for including these in the context menu.
  • I created derived classes from MenuItem for the various spell check options, and styled the text to roughly default WPF. This should allow anyone wishing to override the default look to do so easily.
  • There are two cases where I hard coded text, "Ignore All" and "(no spelling suggestions)". I put this text in the default style for their corresponding menu items. I can see how this might be a bit of a localization issue, and am open to suggestions.

Thoughts?

@ButchersBoy
Copy link
Collaborator

Yes, I definitely want the fix...I will review the code after 2.3.0 is out the door though. 👍

@ButchersBoy ButchersBoy added this to the 2.3.1 milestone Apr 18, 2017
@ButchersBoy
Copy link
Collaborator

image

Adding image for reference. Looks awesome!

@ButchersBoy
Copy link
Collaborator

This looks good. I'm happy with the implementation apart from the derived MenuItem classes. I'm not a big fan of inheritance and wonder if we can get away with this by using normal MenuItems...I guess the down side is we have to look up the style...which might have a certain fragility. But anything increasing the surface API really has to prove its worth to me...

@Keboo
Copy link
Member Author

Keboo commented Apr 18, 2017

I was just trying to make it easy to style those items. However, I suppose I could put those styles as attached properties on the ContextMenu itself that way they could be swapped out if someone wanted to change the styles.

@ButchersBoy
Copy link
Collaborator

ButchersBoy commented Apr 18, 2017

Could we try giving them style keys, a bit like toolbars? This would be more in line with standard WPF and lose the inheritance.

<Style x:Key="{x:Static ToolBar.ButtonStyleKey}" TargetType="Button">

@Keboo
Copy link
Member Author

Keboo commented Apr 18, 2017

Sure, I will update the PR in a few.

@ButchersBoy ButchersBoy merged commit 7f7826a into MaterialDesignInXAML:master Apr 19, 2017
@ButchersBoy
Copy link
Collaborator

Thank-you, nice work!

@Keboo Keboo deleted the spellCheck branch April 19, 2017 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants