Skip to content

Conversation

@nicolaihenriksen
Copy link
Contributor

@nicolaihenriksen nicolaihenriksen commented Aug 20, 2022

Possible fix for #2722.

UPDATE There is another PR (#2824) which refactors this part. This PR only addresses the single problem mentioned in issue #2722 but @Xaalek pointed out that the DataGrid mouse behavior in general was kinda strange. Thus I ended up doing the other PR, just to see if I could make the UX of the DataGrid mouse/keyboard interactions a little smoother and intuitive. Have a look at the other one first, and if that is accepted, this PR should not be needed.


@Xaalek Could you please verify if this is the desired behavior?

The change is quite simple, we bail out of the (tunneling) event PreviewMouseLeftButtonDown in case the DataGridCell receiving the click is already editing. This way the cell itself handles the (bubbling) event MouseLeftButtonDown and does what it needs to. I think this is what is desired.

The effect you get when you single click the column is that cell editing starts and the I-cursor is placed at the position of the click. If you double-click, it first starts the editing and then the "second click" (handled by the cell itself) ends up highlighting/selecting the word at the position of the mouse. Seems intuitive to me, but let me know what you think.

@nicolaihenriksen
Copy link
Contributor Author

@Keboo I also added you as a reviewer just to see if you have any feedback about potential side-effects of this.

@Keboo
Copy link
Member

Keboo commented Aug 22, 2022

No objection from me. @Xaalek it looks like you might have had feedback. Any issues with this one?

@nicolaihenriksen
Copy link
Contributor Author

No objection from me. @Xaalek it looks like you might have had feedback. Any issues with this one?

@Keboo The issues that @Xaalek mentioned he actually did a screen capture of. You can see the link in his comment here. The other PR (#2824) mentioned above is an attempt to address these issues.

@Keboo Keboo merged commit 6a08d55 into MaterialDesignInXAML:master Aug 30, 2022
@Keboo Keboo added this to the 4.6.0 milestone Aug 30, 2022
@nicolaihenriksen nicolaihenriksen deleted the fix2722 branch August 30, 2022 07:43
Keboo added a commit that referenced this pull request Sep 9, 2022
…#2828)

* Initial PasswordBox "revealed" styles

There are still issues with the positioning of the floating hint when password is revealed but empty; the hint should float up, but it does not.

* Fixed alignment and initial wiring up

Floating hint still misplaced when password is empty and revealed

* Showcasing style in sample app + alignment of reveal button

Reveal button aligns with the "clear text" button for consistency. Although I think the "clear text" button is floating a little too high in 2 of the styles (but that is the same for the normal PasswordBox and TextBox styles)

* Showcase that password can still be set from XAML

* Change PasswordChanged event handler to weak event pattern

Avoid leaking memory by replacing the regular event handler with a weak event handler.

* Updated showcasing in the demo tool with a bit more stuff

* Fixed CA warning

* Added UI test for 3-way binding

* [Icon update detected by Github Action]. Auto generated pull request. (#2830)

Co-authored-by: Material Design Service Account <[email protected]>

* Cursor fix (#2832)

Respects a cursor set from outside, otherwise falls back to the desired defaults

* Fixed embedded dialog host style (#2826) (#2829)

Using materialDesign:TransitionAssist.DisableTransitions="true" duration of animation for content cover opacity should be set to 0 to skip animations at all on close dialog host

* Fix for 2596 - Align left margin for error message on outlined TextBox with helper text left margin (#2820)

* Fix left margin for error message on outlined TextBox

The error message and helper text should have the same Margin.Left value for the outlined TextBox.

* Adjusted error text left margin for filled text fields

ComboBoxes, DatePicker and TimePicker all rely on the fields internally, and thus inherit these changes.

* Added UI test for the filled style as well

* Retain original bottom margin

I want to make as few changes as possible to the original style (margin)

* Let DataGridCell handle mouse events itself when already editing (#2821)

* Refactoring DataGridAssist input handling (#2824)

* Refactoring DataGridAssist mouse handling

Rather than handling the tunneling event and "pre-processing", it now handles the (already handled - by the cell) bubbling event to perform additional actions.

* Added template column example

Fixed issue where it would not switch to edit on click

Co-authored-by: Kevin Bost <[email protected]>

* Initial PasswordBox "revealed" styles

There are still issues with the positioning of the floating hint when password is revealed but empty; the hint should float up, but it does not.

* Fixed alignment and initial wiring up

Floating hint still misplaced when password is empty and revealed

* Showcasing style in sample app + alignment of reveal button

Reveal button aligns with the "clear text" button for consistency. Although I think the "clear text" button is floating a little too high in 2 of the styles (but that is the same for the normal PasswordBox and TextBox styles)

* Showcase that password can still be set from XAML

* Change PasswordChanged event handler to weak event pattern

Avoid leaking memory by replacing the regular event handler with a weak event handler.

* Updated showcasing in the demo tool with a bit more stuff

* Fixed CA warning

* Added UI test for 3-way binding

* Resolving merge conflicts

* Resolving merge conflicts

* Fixing hint proxy issues with password box

* Moved samples and added SMTX wrapper

Also fixed a bug regarding the opacity of the PART_ClearButton which was not consistent across the styles.

* Apply cursor fix

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Material Design Service Account <[email protected]>
Co-authored-by: Andrey Nasonov <[email protected]>
Co-authored-by: Kevin Bost <[email protected]>
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.

3 participants