Skip to content

Support keyboard tooltips in TreeNode#4466

Merged
RussKie merged 7 commits intodotnet:mainfrom
SergeySmirnov-Akvelon:Issue-4409_TreeNode_has_no_keyboard_tooltip
Feb 22, 2021
Merged

Support keyboard tooltips in TreeNode#4466
RussKie merged 7 commits intodotnet:mainfrom
SergeySmirnov-Akvelon:Issue-4409_TreeNode_has_no_keyboard_tooltip

Conversation

@SergeySmirnov-Akvelon
Copy link
Contributor

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon commented Jan 14, 2021 •
edited
Loading

Fixes #4439

Proposed changes

  • Added support for "IKeyboardToolTip" interface in "TreeNode" class.
  • Added logic to show the tooltip for the focused TreeNode when the user switches between items

Customer Impact

Before:
4439-before

After:
4439-after

Regression?

  • No

Risk

  • Minimal

Test methodology

  • Manually

Test environment(s)

  • Microsoft Windows [Version 10.0.19041.388]
  • .NET Core 5.0.100-rc.2.20479.15
Microsoft Reviewers: Open in CodeFlow

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon requested a review from a team as a code owner January 14, 2021 10:28
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon marked this pull request as draft January 14, 2021 10:37
@codecov
Copy link

codecov bot commented Jan 14, 2021 •
edited
Loading

Codecov Report

Merging #4466 (e45ae7b) into main (413936e) will increase coverage by 0.01394%.
The diff coverage is 100.00000%.

@@                 Coverage Diff                 @@
##                main       #4466         +/-   ##
===================================================
+ Coverage   97.95443%   97.96837%   +0.01393%     
===================================================
  Files            540         542          +2     
  Lines         263008      263532        +524     
  Branches        4919        4937         +18     
===================================================
+ Hits          257628      258178        +550     
+ Misses          4500        4478         -22     
+ Partials         880         876          -4
Flag Coverage Δ
Debug 97.96837% <100.00000%> (+0.01393%) ⬆️
test 97.96837% <100.00000%> (+0.01393%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch 2 times, most recently from ea5d89e to b078b52 Compare January 14, 2021 11:14
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon marked this pull request as ready for review January 14, 2021 11:14
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch from b078b52 to e0605ee Compare January 15, 2021 08:24
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon added the waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) label Jan 18, 2021
@SergeySmirnov-Akvelon
[Accessibility] Tree node has no keyboard tooltip dotnet#4439
ce3c754 
The issue is reproduced, because when user navigates using keyboard, the message for displaying of the tooltip is not called.

Added support for "IKeyboardToolTip" interface in "TreeNode" class. Added logic to show the tooltip for the focused TreeNode when the user switches between items
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch from 44e9567 to ce3c754 Compare January 26, 2021 06:48
vladimir-krestov
vladimir-krestov reviewed Jan 26, 2021
View reviewed changes
Copy link
Contributor

@vladimir-krestov vladimir-krestov left a comment

Choose a reason for hiding this comment

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

Good work! But it needs to be reworked

vladimir-krestov
vladimir-krestov reviewed Jan 27, 2021
View reviewed changes
Copy link
Contributor

@vladimir-krestov vladimir-krestov left a comment

Choose a reason for hiding this comment

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

Additional points

@RussKie RussKie changed the title [Accessibility] Tree node has no keyboard tooltip #4439 Support keyboard tooltips in TreeNode Jan 29, 2021
RussKie
RussKie reviewed Jan 29, 2021
View reviewed changes
@RussKie RussKie added the waiting-author-feedback The team requires more information from the author label Jan 29, 2021
@vladimir-krestov vladimir-krestov mentioned this pull request Jan 29, 2021
Merged
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Jan 29, 2021
Base automatically changed from master to main February 1, 2021 04:50
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch from 342e7b6 to 8f8c702 Compare February 1, 2021 08:53
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon added the waiting-review This item is waiting on review by one or more members of team label Feb 2, 2021
RussKie
RussKie reviewed Feb 4, 2021
View reviewed changes
@RussKie RussKie added waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Feb 4, 2021
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch from 8f8c702 to d7ef874 Compare February 4, 2021 12:23
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Feb 4, 2021
RussKie
RussKie suggested changes Feb 8, 2021
View reviewed changes
Copy link
Contributor

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

Tests for the changes? E.g. for GetToolInfoWrapper, GetChildNodes

@ghost ghost added the waiting-author-feedback The team requires more information from the author label Feb 8, 2021
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch from d7ef874 to ae4427f Compare February 8, 2021 08:46
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Feb 8, 2021
vladimir-krestov
vladimir-krestov reviewed Feb 8, 2021
View reviewed changes
Copy link
Contributor

@vladimir-krestov vladimir-krestov left a comment

Choose a reason for hiding this comment

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

Little refactoring notes

@RussKie RussKie added the waiting-author-feedback The team requires more information from the author label Feb 9, 2021
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Feb 9, 2021
RussKie
RussKie reviewed Feb 9, 2021
View reviewed changes
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch 2 times, most recently from 3c493ac to 301e184 Compare February 9, 2021 12:50
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch from 301e184 to 6e6ebbb Compare February 9, 2021 14:21
RussKie
RussKie reviewed Feb 15, 2021
View reviewed changes
Copy link
Contributor

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

👍 with few nits

@RussKie RussKie added the waiting-author-feedback The team requires more information from the author label Feb 16, 2021
@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch from 5ab5b91 to 36d4a75 Compare February 16, 2021 14:10
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Feb 16, 2021
RussKie
RussKie reviewed Feb 17, 2021
View reviewed changes
src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/TreeViewTests.cs Outdated
Copy link
Contributor

@RussKie RussKie Feb 17, 2021

Choose a reason for hiding this comment

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

Suggested change
Assert.True(KeyboardToolTipStateMachine.Instance.TestAccessor().IsToolTracked(treeNode1));
var accessor = KeyboardToolTipStateMachine.Instance.TestAccessor();
Assert.True(accessor.IsToolTracked(treeNode1));

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch from 36d4a75 to e9f2a7d Compare February 17, 2021 07:33
RussKie
RussKie reviewed Feb 17, 2021
View reviewed changes
...sts/UnitTests/System/Windows/Forms/TestAccessors.KeyboardToolTipStateMachineTestAccessors.cs Outdated
Copy link
Contributor

@RussKie RussKie Feb 17, 2021

Choose a reason for hiding this comment

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

Suggested change
internal class KeyboardToolTipStateMachineTestAccessors : TestAccessor<KeyboardToolTipStateMachine>
internal class KeyboardToolTipStateMachineTestAccessor : TestAccessor<KeyboardToolTipStateMachine>

Copy link
Contributor Author

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon Feb 17, 2021

Choose a reason for hiding this comment

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

Thank you. Fixed typos

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon force-pushed the Issue-4409_TreeNode_has_no_keyboard_tooltip branch from e9f2a7d to e45ae7b Compare February 17, 2021 08:13
@SergeySmirnov-Akvelon
Copy link
Contributor Author

SergeySmirnov-Akvelon commented Feb 19, 2021

CTI approved

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon removed the waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) label Feb 19, 2021
@RussKie RussKie merged commit 77dcb4f into dotnet:main Feb 22, 2021
@RussKie RussKie deleted the Issue-4409_TreeNode_has_no_keyboard_tooltip branch February 22, 2021 09:29
@ghost ghost added this to the 6.0 Preview2 milestone Feb 22, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@vladimir-krestov vladimir-krestov vladimir-krestov left review comments

@RussKie RussKie RussKie approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon

Labels

None yet

Projects

None yet

Milestone

6.0 Preview2

Development

Successfully merging this pull request may close these issues.

[Accessibility] Tree node has no keyboard tooltip

3 participants

@SergeySmirnov-Akvelon @RussKie @vladimir-krestov