Skip to content

Adding the ability to get keyboard toolTips for TabPages of a TabControl#4421

Merged
RussKie merged 3 commits intodotnet:mainfrom
vladimir-krestov:dev/v-vlkres/TabControlToolTips
Mar 16, 2021
Merged

Adding the ability to get keyboard toolTips for TabPages of a TabControl#4421
RussKie merged 3 commits intodotnet:mainfrom
vladimir-krestov:dev/v-vlkres/TabControlToolTips

Conversation

@vladimir-krestov
Copy link
Contributor

@vladimir-krestov vladimir-krestov commented Jan 1, 2021 •
edited
Loading

Fixes #2717
Related Issue #4305
Based on PR #2719. But the changes were significantly reworked so I have created this new PR
Most of review points from #2719 were fixed here

Proposed changes

  • Notify KeyboardToolTipStateMachine about getting and losing a focus
  • Inverted incorrect dependencies inside ToolTip class

Customer Impact

  • A user can see a TabPage toolTip when using a keyboard

Regression?

  • No

Risk

  • Minimal

Screenshots

Before

  • TabControl shows mouse toolTips only:
    before

After

  • Keyboard toolTips are available for TabControl:
    after

Test methodology

  • Manual testing
  • Unit tests
  • CTI

Test environment(s)

  • .NET Version: 6.0.0-alpha.1.20554.7
  • Microsoft Windows [Version 10.0.19042.685]
Microsoft Reviewers: Open in CodeFlow

@vladimir-krestov vladimir-krestov requested a review from a team as a code owner January 1, 2021 14:46
@ghost ghost assigned vladimir-krestov Jan 1, 2021
@vladimir-krestov vladimir-krestov marked this pull request as draft January 1, 2021 14:50
RussKie
RussKie reviewed Jan 4, 2021
View reviewed changes
RussKie
RussKie reviewed Jan 4, 2021
View reviewed changes
@RussKie RussKie added the waiting-author-feedback The team requires more information from the author label Jan 4, 2021
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Jan 5, 2021
@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch 2 times, most recently from 3e38e04 to 9d57993 Compare January 5, 2021 20:32
RussKie
RussKie reviewed Jan 6, 2021
View reviewed changes
@codecov
Copy link

codecov bot commented Jan 6, 2021 •
edited
Loading

Codecov Report

Merging #4421 (d6ffe22) into main (fcc6b67) will increase coverage by 0.00979%.
The diff coverage is 100.00000%.

@@                 Coverage Diff                 @@
##                main       #4421         +/-   ##
===================================================
+ Coverage   97.96388%   97.97367%   +0.00978%     
===================================================
  Files            543         543                 
  Lines         264375      264666        +291     
  Branches        4970        5007         +37     
===================================================
+ Hits          258992      259303        +311     
+ Misses          4499        4483         -16     
+ Partials         884         880          -4
Flag Coverage Δ
Debug 97.97367% <100.00000%> (+0.00978%) ⬆️
production 100.00000% <ø> (?)
test 97.97367% <100.00000%> (+0.00978%) ⬆️

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

@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch 2 times, most recently from 1169aca to 23ae6a3 Compare January 25, 2021 09:40
Base automatically changed from master to main February 1, 2021 04:50
@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch from 23ae6a3 to 09e8c85 Compare February 14, 2021 14:33
@vladimir-krestov vladimir-krestov added the waiting-review This item is waiting on review by one or more members of team label Feb 14, 2021
@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch from b5761fb to 5de450e Compare February 18, 2021 08:16
@vladimir-krestov vladimir-krestov marked this pull request as ready for review February 18, 2021 09:04
SergeySmirnov-Akvelon
SergeySmirnov-Akvelon reviewed Feb 18, 2021
View reviewed changes
Copy link
Contributor

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon left a comment

Choose a reason for hiding this comment

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

Nice job!!! I have added several review points, most of which are cosmetic

@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch from 5de450e to 1d87a51 Compare February 19, 2021 18:09
@vladimir-krestov vladimir-krestov 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 Feb 28, 2021
RussKie
RussKie suggested changes Mar 1, 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

@ghost ghost added the waiting-author-feedback The team requires more information from the author label Mar 1, 2021
@RussKie RussKie removed the waiting-review This item is waiting on review by one or more members of team label Mar 1, 2021
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Mar 2, 2021
@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch from 1d87a51 to 7e355ef Compare March 2, 2021 18:52
@vladimir-krestov
Copy link
Contributor Author

vladimir-krestov commented Mar 2, 2021

Working on test issues...

@vladimir-krestov
Copy link
Contributor Author

vladimir-krestov commented Mar 3, 2021 •
edited
Loading

Hi @merriemcgaw, could you please take a look at thу following cases:

We have a TabControl with 2 pages.
TabPage has ToolTipText property that sets internal toolTip for the "tab".
Also, we can set an external ToolTip for a "page".
image

After my fix we have the following cases:

  • Case 1:
    ToolTipText is set for the tabPage21 - "Internal tooltip for TabPage1" (an external toolTip is set too with "External tooltip for TabPage1" text) .
    An external ToolTip is set (toolTip.SetToolTip(...)) for tabPage22 (ToolTipText is not set) - "External tooltip for TabPage2".
    So, keyboard toolTips look like that (using keyboard arrows):
    HEfVSt6qni

    Here tabPage21 shows ToolTipText, not the external ToolTip text because ToolTipText has priority for keyboard.
    Question 1: is it correct?

  • Case 2:
    An external ToolTip is set for tabPage22 (using the mouse):
    zKNQyYfaOx

    There is no a toolTip if the mouse hovers on "tab", and a toolTip appears if the mouse hovers on "page". This is native behavior.
    But I made, that "tab" will show the toolTip if we select it using a keyboard:
    KDu91DCHpx

    Question 2: is it correct?

  • Case 3:
    An external ToolTip is set for the TabControl - "External tooltip for TabControl":
    fo1mcCANzb

    Here the toolTip doesn't appear if the mouse hovers on "page". And the toolTip appears for "tabs" only. This is native behavior.
    I decided to make the following for keyboard toolTips:

    1. the toolTip doesn't appear if we select some tabs
    2. the toolTip appears if we focus on the TabControl from another control:
      5ePssuRLl6

    And if a toolTip is set for a TabPage then when we focus on the TabControl from another control we will see the TabPage toolTip
    qBiPL8jGnU

    Question 3: is it correct?

@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch 2 times, most recently from 6547e3c to 11dfafd Compare March 5, 2021 09:58
@vladimir-krestov vladimir-krestov added waiting-review This item is waiting on review by one or more members of team and removed waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) labels Mar 9, 2021
@vladimir-krestov
Copy link
Contributor Author

vladimir-krestov commented Mar 9, 2021

Testers approved the fix ✔️

@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch from 11dfafd to 5083708 Compare March 9, 2021 11:29
@vladimir-krestov
Copy link
Contributor Author

vladimir-krestov commented Mar 9, 2021

@RussKie, @dreddy-work, please do final review.

RussKie
RussKie suggested changes Mar 10, 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

@ghost ghost added the waiting-author-feedback The team requires more information from the author label Mar 10, 2021
@RussKie RussKie removed the waiting-review This item is waiting on review by one or more members of team label Mar 10, 2021
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Mar 10, 2021
@RussKie
Copy link
Contributor

RussKie commented Mar 10, 2021

Have you pushed the changes?

@RussKie RussKie added the waiting-author-feedback The team requires more information from the author label Mar 10, 2021
@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch from 5083708 to 19463bd Compare March 11, 2021 04:19
@ghost ghost removed the waiting-author-feedback The team requires more information from the author label Mar 11, 2021
@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch 2 times, most recently from f4a7156 to 80e31dd Compare March 12, 2021 04:31
@vladimir-krestov vladimir-krestov added the waiting-review This item is waiting on review by one or more members of team label Mar 12, 2021
@vladimir-krestov
Added keyboard tooltips to TabPages
3e9780c 
Fixes dotnet#2717
@vladimir-krestov
Inverted incorrect dependencies inside ToolTip
ee34576 
The current implementation introduces a hard coupling between ToolTip and some controls, which is extremely bad.
These changes removed these unreliable dependencies
@vladimir-krestov
Added maui tests for TabControl
2120b63
@vladimir-krestov vladimir-krestov force-pushed the dev/v-vlkres/TabControlToolTips branch from 80e31dd to 2120b63 Compare March 16, 2021 15:26
@RussKie RussKie removed the waiting-review This item is waiting on review by one or more members of team label Mar 16, 2021
@RussKie RussKie merged commit 325507e into dotnet:main Mar 16, 2021
@ghost ghost added this to the 6.0 Preview3 milestone Mar 16, 2021
@Olina-Zhang Olina-Zhang mentioned this pull request Mar 31, 2021
Closed
@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

@dreddy-work dreddy-work Awaiting requested review from dreddy-work

2 more reviewers

@SergeySmirnov-Akvelon SergeySmirnov-Akvelon SergeySmirnov-Akvelon left review comments

@RussKie RussKie RussKie approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@vladimir-krestov vladimir-krestov

Labels

None yet

Projects

None yet

Milestone

6.0 Preview3

Development

Successfully merging this pull request may close these issues.

[Accessibility] TabPage has no keyboard tooltip

3 participants

@vladimir-krestov @RussKie @SergeySmirnov-Akvelon