Skip to content

Don't display empty tooltips (Tooltips with empty message property)#87638

Merged
fluttergithubbot merged 9 commits into
flutter:masterfrom
bambinoua:master
Sep 10, 2021
Merged

Don't display empty tooltips (Tooltips with empty message property)#87638
fluttergithubbot merged 9 commits into
flutter:masterfrom
bambinoua:master

Conversation

@bambinoua

@bambinoua bambinoua commented Aug 4, 2021

Copy link
Copy Markdown
Contributor

If tooltip message is empty than no need to display empty dark rectangle.

@bambinoua
Update tooltip.dart (lines 506-511)
ad805b3 
If tooltip message is empty than no need to display empty dark rectangle.
@flutter-dashboard

flutter-dashboard Bot commented Aug 4, 2021

Copy link
Copy Markdown

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@flutter-dashboard flutter-dashboard Bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Aug 4, 2021
@google-cla google-cla Bot added the cla: yes label Aug 4, 2021
@bambinoua
Update tooltip.dart
8e554d8 
Remove trailingg spaces
@bambinoua bambinoua mentioned this pull request Aug 4, 2021
Closed
@dkwingsmt

dkwingsmt commented Aug 6, 2021

Copy link
Copy Markdown
Contributor

The change looks ok. Can you add a unit test so that someone doesn't accidentally undo it?

@HansMuller HansMuller requested a review from dkwingsmt August 11, 2021 15:43
@royarg02

royarg02 commented Aug 26, 2021

Copy link
Copy Markdown
Contributor

If this is merged, we could do away with Chip.useDeleteButtonTooltip added in #68237 (I happen to know this one since I authored it).

@dkwingsmt

dkwingsmt commented Aug 26, 2021

Copy link
Copy Markdown
Contributor

@bambinoua Can you keep this PR updated?

@bambinoua
Merge branch 'flutter:master' into master
46d3979
@bambinoua
Update tooltip_test.dart
c007152 
Add additional unit tests from line 1501 to 1530
@bambinoua

bambinoua commented Aug 31, 2021

Copy link
Copy Markdown
Contributor Author

The change looks ok. Can you add a unit test so that someone doesn't accidentally undo it?

@dkwingsmt I've added tests but I did not do them ever before. Please check I created acceptable ones.

@bambinoua
Update tooltip_test.dart
f26c1ec 
Remove some errors
@bambinoua
Update tooltip_test.dart
0066447 
Try to fix trailing spaces.
@bambinoua
Update tooltip_test.dart
97434c9 
Add const constructors
@bambinoua
Update tooltip_test.dart
a78f352 
Fix const constructor
@bambinoua

bambinoua commented Sep 1, 2021

Copy link
Copy Markdown
Contributor Author

I do not know why some checks were not successful

dkwingsmt
dkwingsmt approved these changes Sep 10, 2021
View reviewed changes

@dkwingsmt dkwingsmt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dkwingsmt

dkwingsmt commented Sep 10, 2021

Copy link
Copy Markdown
Contributor

If this is merged, we could do away with Chip.useDeleteButtonTooltip added in #68237 (I happen to know this one since I authored it).

That's a very reasonable idea. Be aware that the change might be breaking or at least need a migration guide.

@fluttergithubbot fluttergithubbot merged commit 7736b9e into flutter:master Sep 10, 2021
@royarg02 royarg02 mentioned this pull request Sep 21, 2021
Closed
@victorsanni victorsanni mentioned this pull request Feb 18, 2026
Merged
github-merge-queue Bot pushed a commit that referenced this pull request Feb 21, 2026
@victorsanni
Shortcircuit if Tooltip message and richMessage are empty (#182524)
e6e3696 
Regressed when migrating to `RawTooltip`. 

The unit tests added in #87638 to
prevent this from happening were faulty, I've updated them to reflect
this feature.

Fixes [[flutter 3.41.x] NavigationDestination shows empty
tooltip](#182495)
ahmedsameha1 pushed a commit to ahmedsameha1/flutter that referenced this pull request Feb 27, 2026
@victorsanni @ahmedsameha1
Shortcircuit if Tooltip message and richMessage are empty (flutter#18…
ec94d7c 
…2524)

Regressed when migrating to `RawTooltip`. 

The unit tests added in flutter#87638 to
prevent this from happening were faulty, I've updated them to reflect
this feature.

Fixes [[flutter 3.41.x] NavigationDestination shows empty
tooltip](flutter#182495)
mboetger pushed a commit to mboetger/flutter that referenced this pull request Mar 26, 2026
@victorsanni @mboetger
Shortcircuit if Tooltip message and richMessage are empty (flutter#18…
2c66893 
…2524)

Regressed when migrating to `RawTooltip`. 

The unit tests added in flutter#87638 to
prevent this from happening were faulty, I've updated them to reflect
this feature.

Fixes [[flutter 3.41.x] NavigationDestination shows empty
tooltip](flutter#182495)
chunhtai pushed a commit to chunhtai/packages that referenced this pull request Jun 12, 2026
@victorsanni
Shortcircuit if Tooltip message and richMessage are empty (#182524)
9a7f259 
Regressed when migrating to `RawTooltip`. 

The unit tests added in flutter/flutter#87638 to
prevent this from happening were faulty, I've updated them to reflect
this feature.

Fixes [[flutter 3.41.x] NavigationDestination shows empty
tooltip](flutter/flutter#182495)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dkwingsmt dkwingsmt dkwingsmt approved these changes

Assignees

No one assigned

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bambinoua @dkwingsmt @royarg02 @fluttergithubbot