Tooltip docs: Recommend setting preferBelow to false in theme #135879
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
framework
This was referenced Oct 2, 2023
Contributor
Author
Contributor
Author
|
I see some CI output about the updated example code and plan to address it as soon as Wednesday. |
chrisbobbe
force-pushed
the
pr-prefer-below-docs-change
branch
from
5c37a86 to
63bddf7
Compare
Contributor
Author
|
OK, made a change that should resolve it; I'll check back and investigate if it doesn't. |
chrisbobbe
force-pushed
the
pr-prefer-below-docs-change
branch
from
63bddf7 to
18a5992
Compare
Contributor
Author
|
OK, all checks have passed and this is ready for review; PTAL. 🙂 |
gnprice
reviewed
Oct 11, 2023
Member
gnprice
left a comment
gnprice
left a comment
There was a problem hiding this comment.
Thanks for taking care of this! Small comments.
Comment on lines
-234
to
+250
Member
There was a problem hiding this comment.
Ah cool, good to add this point which had been missing.
Member
There was a problem hiding this comment.
In the next example (tooltip.0.dart), the example is meant to illustrate the panoply of options available, but it now ends up with the same effective preferBelow value as the other examples.
Perhaps the cleanest thing there would be to have it say preferBelow: false up in a ThemeData at the top (just like the other examples — and so, like them, setting the example that people should typically follow when not constrained by an existing codebase), and then preferBelow: true on the individual Tooltip, so that it effectively demos the option.
Contributor
Author
There was a problem hiding this comment.
In the next example (
tooltip.0.dart) […]
I think you mean tooltip.1.dart?
Comment on lines
252
to
253
Member
There was a problem hiding this comment.
Hmm, it actually seems a bit nontrivial to work out from this, in the context of the existing docs, how one can go about following this advice.
From [TooltipThemeData.preferBelow] one can naturally follow a link to [TooltipThemeData]. But then it's not clear from there what one does with a [TooltipThemeData] to make it take effect.
Well, I guess from that one learns about [TooltipTheme] widgets, and can use one of those. But I think the idiomatic thing, at least when applying it to the whole app, is probably to wrap it up in a [ThemeData] and use either a [Theme] widget or [MaterialApp.theme].
I guess maybe the simplest solution is if I send my own small PR adding a couple of appropriate cross-references there. This advice is still helpful, in that people can use TooltipTheme, or look at the examples above, or just already have a mental picture of the Material "theme" system and so already know where to slot in this added fact.
Member
There was a problem hiding this comment.
Sent #137316 with those other cross-references, and some other tweaks I found I wanted to make once I started editing. (All still orthogonal to this PR, I think.)
Comment on lines
71
to
73
Member
There was a problem hiding this comment.
The style guide says to avoid "you":
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#use-the-passive-voice-recommend-do-not-require-never-say-things-are-simple
Can just s/your/the/, I think.
chrisbobbe
force-pushed
the
pr-prefer-below-docs-change
branch
from
1c9e55e to
57242c0
Compare
Contributor
Author
|
Thanks for the review @gnprice! Revision pushed. For #135879 (comment), would that be something you'd like to do as a followup after this PR settles? |
gnprice
reviewed
Oct 26, 2023
Member
There was a problem hiding this comment.
Thanks! All LGTM except one small comment below.
For #135879 (comment), would that be something you'd like to do as a followup after this PR settles?
Yeah, I think it's orthogonal to the changes in this PR. (→ #137316)
Comment on lines
-40
to
+41
Member
There was a problem hiding this comment.
There's a bit that needs to be updated for this in the doc that points to this example:
/// `preferBelow` is false, the tooltip will prefer showing above [Tooltip]'s child widget.
/// However, it may show the tooltip below if there's not enough space
/// above the widget.
gnprice
added a commit
to gnprice/flutter
that referenced
this pull request
Oct 26, 2023
Much of the new wording here is borrowed from [ChipTheme], [SliderTheme], or [RadioThemeData], which I think are pretty good. I believe a lot of other theme classes have similar wording too. I've also made some tweaks of my own, notably the references to [MaterialApp.theme]. This started from a desire to have clearer cross-references pointing at what to do with a FooThemeData to make it take effect: flutter#135879 (comment) but then as I started writing I kept finding more and more small things I wanted to adjust, including a couple of bits that were extraneous or obsolete.
8 tasks
chrisbobbe
force-pushed
the
pr-prefer-below-docs-change
branch
from
57242c0 to
64c5404
Compare
Contributor
Author
|
Thanks for the review! Revision pushed. |
gnprice
approved these changes
Nov 3, 2023
Member
gnprice
left a comment
gnprice
left a comment
There was a problem hiding this comment.
Looks good; marking for merge. Thanks again!
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 4, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 4, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 4, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 5, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 5, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 6, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 6, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 6, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 6, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 6, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Nov 6, 2023
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Nov 6, 2023
Roll Flutter from 29b2516 to f5a9835 (101 revisions) flutter/flutter@29b2516...f5a9835 2023-11-06 [email protected] Check sample links for malformed links (flutter/flutter#137807) 2023-11-06 [email protected] Change cast in json parsing (flutter/flutter#137708) 2023-11-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Update BottomNavigationBar tests for M3" (flutter/flutter#137948) 2023-11-06 [email protected] Roll Packages from cccf5d2 to 49eac1f (2 revisions) (flutter/flutter#137943) 2023-11-06 [email protected] Update BottomNavigationBar tests for M3 (flutter/flutter#136624) 2023-11-06 [email protected] Roll Flutter Engine from 4f6ed31bd8bd to bdfa8aa8f81f (1 revision) (flutter/flutter#137941) 2023-11-06 [email protected] Roll Flutter Engine from b9b3269b0b2c to 4f6ed31bd8bd (2 revisions) (flutter/flutter#137935) 2023-11-06 [email protected] Provide a helpful error message when `ColorScheme.brightness` doesn't match `ThemeData.brightness` (flutter/flutter#137611) 2023-11-06 [email protected] Roll Flutter Engine from 555ffa17b55c to b9b3269b0b2c (1 revision) (flutter/flutter#137933) 2023-11-06 [email protected] Fix tool exit message shown when user provides a non-list to "assets" for a deferred component (flutter/flutter#137837) 2023-11-06 [email protected] Roll Flutter Engine from 0d8c7ceacc01 to 555ffa17b55c (1 revision) (flutter/flutter#137921) 2023-11-06 [email protected] Roll Flutter Engine from 11d66db97d3f to 0d8c7ceacc01 (1 revision) (flutter/flutter#137920) 2023-11-05 [email protected] Roll Flutter Engine from a7592e42464c to 11d66db97d3f (1 revision) (flutter/flutter#137914) 2023-11-05 [email protected] Roll Flutter Engine from 1c6bd97e2288 to a7592e42464c (1 revision) (flutter/flutter#137912) 2023-11-05 [email protected] Roll Flutter Engine from daf18fe46b72 to 1c6bd97e2288 (1 revision) (flutter/flutter#137908) 2023-11-04 [email protected] Roll Flutter Engine from a45e679828e6 to daf18fe46b72 (1 revision) (flutter/flutter#137904) 2023-11-04 [email protected] Roll Flutter Engine from fb2a9c20141e to a45e679828e6 (1 revision) (flutter/flutter#137903) 2023-11-04 [email protected] Roll Flutter Engine from 576833873c15 to fb2a9c20141e (1 revision) (flutter/flutter#137900) 2023-11-04 [email protected] Roll Flutter Engine from 25f5e285f874 to 576833873c15 (1 revision) (flutter/flutter#137898) 2023-11-04 [email protected] Roll Flutter Engine from 7282a5d94ab6 to 25f5e285f874 (2 revisions) (flutter/flutter#137892) 2023-11-04 [email protected] Roll Flutter Engine from b66a87626300 to 7282a5d94ab6 (2 revisions) (flutter/flutter#137887) 2023-11-04 [email protected] HeroController should dispatch creation and disposal events. (flutter/flutter#137835) 2023-11-04 [email protected] Roll Flutter Engine from ec20731de6ff to b66a87626300 (1 revision) (flutter/flutter#137877) 2023-11-03 [email protected] InheritedElement.removeDependent() (flutter/flutter#129210) 2023-11-03 [email protected] Remove unused generic type from BottomSheet (flutter/flutter#137791) 2023-11-03 [email protected] Roll Flutter Engine from 035740c1f90e to ec20731de6ff (2 revisions) (flutter/flutter#137872) 2023-11-03 [email protected] Pin dart-lang/native dependencies (flutter/flutter#137601) 2023-11-03 [email protected] Send caret rect to embedder on selection update (flutter/flutter#137863) 2023-11-03 [email protected] Roll Flutter Engine from 677040f10f65 to 035740c1f90e (4 revisions) (flutter/flutter#137871) 2023-11-03 [email protected] Tooltip docs: Recommend setting preferBelow to false in theme (flutter/flutter#135879) 2023-11-03 [email protected] Roll Flutter Engine from f363a6e5e093 to 677040f10f65 (2 revisions) (flutter/flutter#137861) 2023-11-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Android] Support Android 34" (flutter/flutter#137865) 2023-11-03 [email protected] InkFeature should dispatch creation and disposal events. (flutter/flutter#137793) 2023-11-03 [email protected] AppLifecycleListener should dispatch creation and disposal events. (flutter/flutter#137840) 2023-11-03 [email protected] Roll Flutter Engine from d5ccb5b1b706 to f363a6e5e093 (2 revisions) (flutter/flutter#137858) 2023-11-03 [email protected] Roll Flutter Engine from 72262a238090 to d5ccb5b1b706 (3 revisions) (flutter/flutter#137857) 2023-11-03 [email protected] Updated the nested navigation NavigationBar example (flutter/flutter#137788) 2023-11-03 [email protected] Roll Flutter Engine from 0415a4f5e2a2 to 72262a238090 (2 revisions) (flutter/flutter#137853) 2023-11-03 [email protected] Roll Flutter Engine from 8531c5935356 to 0415a4f5e2a2 (1 revision) (flutter/flutter#137847) 2023-11-03 [email protected] Roll flutter gallery version forward. (flutter/flutter#137846) 2023-11-03 [email protected] Roll Flutter Engine from 43653c5a3ec8 to 8531c5935356 (1 revision) (flutter/flutter#137845) 2023-11-03 [email protected] Roll Packages from 33c2b4e to cccf5d2 (6 revisions) (flutter/flutter#137841) 2023-11-03 [email protected] [web] dispatch corresponding keyup events in text editing integrations (flutter/flutter#136874) 2023-11-03 [email protected] [leak-tracking] Add more leak tracking in test/painting #3 (flutter/flutter#136170) 2023-11-03 [email protected] Upgrade leak_tracker and remove some deps in allow list. (flutter/flutter#137806) 2023-11-03 [email protected] Roll Flutter Engine from fc7c3f70c076 to 43653c5a3ec8 (1 revision) (flutter/flutter#137827) ...
auto-submit bot
pushed a commit
that referenced
this pull request
Jan 24, 2024
Much of the new wording here is borrowed from [ChipTheme], [SliderTheme], or [RadioThemeData], which I think are pretty good. I believe a lot of other theme classes have similar wording too. I've also made some tweaks of my own, notably the references to [MaterialApp.theme]. This started from a desire to have clearer cross-references pointing at what to do with a FooThemeData to make it take effect: #135879 (comment) but then as I started writing I kept finding more and more small things I wanted to adjust, including a couple of bits that were extraneous or obsolete.
flutteractionsbot
pushed a commit
to flutteractionsbot/flutter
that referenced
this pull request
Jan 24, 2024
…37316) Much of the new wording here is borrowed from [ChipTheme], [SliderTheme], or [RadioThemeData], which I think are pretty good. I believe a lot of other theme classes have similar wording too. I've also made some tweaks of my own, notably the references to [MaterialApp.theme]. This started from a desire to have clearer cross-references pointing at what to do with a FooThemeData to make it take effect: flutter#135879 (comment) but then as I started writing I kept finding more and more small things I wanted to adjust, including a couple of bits that were extraneous or obsolete.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Feb 16, 2024
chrisbobbe
added a commit
to chrisbobbe/zulip-flutter
that referenced
this pull request
Apr 24, 2024
This is now the natural thing to do for tooltipTheme, following recommendations in the upstream docs (thanks to my PR flutter/flutter#135879!) and doesn't need an explicit comment explaining it.
chrisbobbe
added a commit
to chrisbobbe/zulip-flutter
that referenced
this pull request
Apr 27, 2024
This is now the natural thing to do for tooltipTheme, following recommendations in the upstream docs (thanks to my PR flutter/flutter#135879!) and doesn't need an explicit comment explaining it.
chrisbobbe
added a commit
to chrisbobbe/zulip-flutter
that referenced
this pull request
Apr 30, 2024
This is now the natural thing to do for tooltipTheme, following recommendations in the upstream docs (thanks to my PR flutter/flutter#135879!) and doesn't need an explicit comment explaining it.
chrisbobbe
added a commit
to zulip/zulip-flutter
that referenced
this pull request
Apr 30, 2024
This is now the natural thing to do for tooltipTheme, following recommendations in the upstream docs (thanks to my PR flutter/flutter#135879!) and doesn't need an explicit comment explaining it.
dko5ki23t
pushed a commit
to dko5ki23t/google_maps_flutter_improved
that referenced
this pull request
May 24, 2025
Roll Flutter from 29b25165cab8 to f5a983535131 (101 revisions) flutter/flutter@29b2516...f5a9835 2023-11-06 [email protected] Check sample links for malformed links (flutter/flutter#137807) 2023-11-06 [email protected] Change cast in json parsing (flutter/flutter#137708) 2023-11-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Update BottomNavigationBar tests for M3" (flutter/flutter#137948) 2023-11-06 [email protected] Roll Packages from cccf5d24d703 to 49eac1fec6c7 (2 revisions) (flutter/flutter#137943) 2023-11-06 [email protected] Update BottomNavigationBar tests for M3 (flutter/flutter#136624) 2023-11-06 [email protected] Roll Flutter Engine from 4f6ed31bd8bd to bdfa8aa8f81f (1 revision) (flutter/flutter#137941) 2023-11-06 [email protected] Roll Flutter Engine from b9b3269b0b2c to 4f6ed31bd8bd (2 revisions) (flutter/flutter#137935) 2023-11-06 [email protected] Provide a helpful error message when `ColorScheme.brightness` doesn't match `ThemeData.brightness` (flutter/flutter#137611) 2023-11-06 [email protected] Roll Flutter Engine from 555ffa17b55c to b9b3269b0b2c (1 revision) (flutter/flutter#137933) 2023-11-06 [email protected] Fix tool exit message shown when user provides a non-list to "assets" for a deferred component (flutter/flutter#137837) 2023-11-06 [email protected] Roll Flutter Engine from 0d8c7ceacc01 to 555ffa17b55c (1 revision) (flutter/flutter#137921) 2023-11-06 [email protected] Roll Flutter Engine from 11d66db97d3f to 0d8c7ceacc01 (1 revision) (flutter/flutter#137920) 2023-11-05 [email protected] Roll Flutter Engine from a7592e42464c to 11d66db97d3f (1 revision) (flutter/flutter#137914) 2023-11-05 [email protected] Roll Flutter Engine from 1c6bd97e2288 to a7592e42464c (1 revision) (flutter/flutter#137912) 2023-11-05 [email protected] Roll Flutter Engine from daf18fe46b72 to 1c6bd97e2288 (1 revision) (flutter/flutter#137908) 2023-11-04 [email protected] Roll Flutter Engine from a45e679828e6 to daf18fe46b72 (1 revision) (flutter/flutter#137904) 2023-11-04 [email protected] Roll Flutter Engine from fb2a9c20141e to a45e679828e6 (1 revision) (flutter/flutter#137903) 2023-11-04 [email protected] Roll Flutter Engine from 576833873c15 to fb2a9c20141e (1 revision) (flutter/flutter#137900) 2023-11-04 [email protected] Roll Flutter Engine from 25f5e285f874 to 576833873c15 (1 revision) (flutter/flutter#137898) 2023-11-04 [email protected] Roll Flutter Engine from 7282a5d94ab6 to 25f5e285f874 (2 revisions) (flutter/flutter#137892) 2023-11-04 [email protected] Roll Flutter Engine from b66a87626300 to 7282a5d94ab6 (2 revisions) (flutter/flutter#137887) 2023-11-04 [email protected] HeroController should dispatch creation and disposal events. (flutter/flutter#137835) 2023-11-04 [email protected] Roll Flutter Engine from ec20731de6ff to b66a87626300 (1 revision) (flutter/flutter#137877) 2023-11-03 [email protected] InheritedElement.removeDependent() (flutter/flutter#129210) 2023-11-03 [email protected] Remove unused generic type from BottomSheet (flutter/flutter#137791) 2023-11-03 [email protected] Roll Flutter Engine from 035740c1f90e to ec20731de6ff (2 revisions) (flutter/flutter#137872) 2023-11-03 [email protected] Pin dart-lang/native dependencies (flutter/flutter#137601) 2023-11-03 [email protected] Send caret rect to embedder on selection update (flutter/flutter#137863) 2023-11-03 [email protected] Roll Flutter Engine from 677040f10f65 to 035740c1f90e (4 revisions) (flutter/flutter#137871) 2023-11-03 [email protected] Tooltip docs: Recommend setting preferBelow to false in theme (flutter/flutter#135879) 2023-11-03 [email protected] Roll Flutter Engine from f363a6e5e093 to 677040f10f65 (2 revisions) (flutter/flutter#137861) 2023-11-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Android] Support Android 34" (flutter/flutter#137865) 2023-11-03 [email protected] InkFeature should dispatch creation and disposal events. (flutter/flutter#137793) 2023-11-03 [email protected] AppLifecycleListener should dispatch creation and disposal events. (flutter/flutter#137840) 2023-11-03 [email protected] Roll Flutter Engine from d5ccb5b1b706 to f363a6e5e093 (2 revisions) (flutter/flutter#137858) 2023-11-03 [email protected] Roll Flutter Engine from 72262a238090 to d5ccb5b1b706 (3 revisions) (flutter/flutter#137857) 2023-11-03 [email protected] Updated the nested navigation NavigationBar example (flutter/flutter#137788) 2023-11-03 [email protected] Roll Flutter Engine from 0415a4f5e2a2 to 72262a238090 (2 revisions) (flutter/flutter#137853) 2023-11-03 [email protected] Roll Flutter Engine from 8531c5935356 to 0415a4f5e2a2 (1 revision) (flutter/flutter#137847) 2023-11-03 [email protected] Roll flutter gallery version forward. (flutter/flutter#137846) 2023-11-03 [email protected] Roll Flutter Engine from 43653c5a3ec8 to 8531c5935356 (1 revision) (flutter/flutter#137845) 2023-11-03 [email protected] Roll Packages from 33c2b4e53ad7 to cccf5d24d703 (6 revisions) (flutter/flutter#137841) 2023-11-03 [email protected] [web] dispatch corresponding keyup events in text editing integrations (flutter/flutter#136874) 2023-11-03 [email protected] [leak-tracking] Add more leak tracking in test/painting #3 (flutter/flutter#136170) 2023-11-03 [email protected] Upgrade leak_tracker and remove some deps in allow list. (flutter/flutter#137806) 2023-11-03 [email protected] Roll Flutter Engine from fc7c3f70c076 to 43653c5a3ec8 (1 revision) (flutter/flutter#137827) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed at #133007 (comment), this is a docs change meant to help people in the absence of a fix for #133006, which is being closed as WONTFIX.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.