Tweaked TabBar to provide uniform padding to all tabs in cases where only few tabs contain both icon and text #80237
Conversation
0ab16be to
97847d3
Compare
| /// text or icon. | ||
| bool get tabHasTextAndIcon { | ||
| for (final Widget item in tabs) { | ||
| if (item is Tab) { |
There was a problem hiding this comment.
This hard codes specific knowledge of another widget into TabBar, which means the TabBar will behave differently depending on its tabs.
This is something that is discussed in the style guide here: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo/b91c3c62d96814e8fbf7119be65ddeb5a9746f82#avoid-interleaving-multiple-concepts-together
There was a problem hiding this comment.
Hmm, this is true. Although, preferredSize seems to use the same kind of code here:
So I figured it would be okay to treat tabs differently to handle tabs in this special case? 🤔Also, this irregularity arises because the Tabs are treated(sized) differently within a TabBar in the first place.
There was a problem hiding this comment.
Oh wow I had not seen that before. Interesting. I am going to see if I can get a second opinion on this, I was not aware of this exception to the style guide.
Also, this irregularity arises because the Tabs are treated(sized) differently within a TabBar in the first place.
It may be worth investigating if that is where our bug lies.
There was a problem hiding this comment.
It looks like in the linked bug there is a solution to the problem by sizing the Tabs such that they are uniform, I am not sure if that may be a better solution than pressing the style guide further here - but let me see if I can get some more opinions on this.
Also, I did notice you pointed out in the bug that Material Design has an opinion about this, so maybe this is intentional.
| /// text or icon. | ||
| bool get tabHasTextAndIcon { | ||
| for (final Widget item in tabs) { | ||
| if (item is Tab) { |
There was a problem hiding this comment.
Oh wow I had not seen that before. Interesting. I am going to see if I can get a second opinion on this, I was not aware of this exception to the style guide.
Also, this irregularity arises because the Tabs are treated(sized) differently within a TabBar in the first place.
It may be worth investigating if that is where our bug lies.
Actually I did try sizing the tabs itself earlier but that proved to be cumbersome. The problem is that for the tabs to resize themselves they would have to know:
Both of these requires a child widget to know the properties of the parent, which I guess is not possible?🤔 |
There was a problem hiding this comment.
Chatted with @Hixie about this, available here: https://discord.com/channels/608014603317936148/608021234516754444/832710552404230204
(it's ok to check is PreferredSizeWidget since that's an interface)
Per that discussion we should see about changing the TabBar.preferredSize to actually check for the PreferredSize widget, and wrap the Tab in one.
I have filed #80596 for tracking that.
We should investigate using a similar solution here as well since TabBar shouldn't have knowledge of Tabs.
Hey @Piinks, I've updated the PR since #80792 has landed. Please have a look :). |
8f3553c to
178fc1a
Compare
…only few tabs contain both icon and text
Tabs should have uniform active regions regardless of their configuration.
List which issues are fixed by this PR. You must list at least one issue.
Fixes: #80112
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.