Skip to content

Conversation

@TahaTesser
Copy link
Member

@TahaTesser TahaTesser commented May 2, 2023

fixes #123112

Description

  1. Add proper M3 indicator height akaIndictorWeight from the M3 specs for the primary tab bar with label indicator size.
    "md.comp.primary-navigation-tab.active-indicator.height": 3.0,

    (this was held due to indicatorWeight having a hard-coded value)

and added a secondary tab bar indicator height.
2. Set a minimum value for the rounded indicator to maintain the indicator shape.
3. With proper indicator height, the rounded indicator position is also fixed.
4. Fix round indicator is shown for the primary tab bar with tab indicator size.
5. Above changes fix #123112.
6. Fix the startOffset const value from #125036 to match docs and move it to a variable.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels May 2, 2023
@TahaTesser TahaTesser requested a review from Piinks May 2, 2023 14:32
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #125883 at sha e6c8a1acc167eec2a568cbf44e2254cd0d2cc7c3

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label May 2, 2023
@Piinks Piinks added c: new feature Nothing broken; request for a new capability a: fidelity Matching the OEM platforms better a: quality A truly polished experience labels May 2, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it overridden for values < 3.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

This preserves the rounded indicator shape and overrides the 2.0 indicator weight.

this.indicatorWeight = 2.0,

Screenshot 2023-05-03 at 14 17 00

If the value is less than 3.0, this indicator shape cannot be maintained. This has rounded corners, unlike the tab indicator size indicator.

Screenshot 2023-05-03 at 14 14 00

@TahaTesser TahaTesser force-pushed the fix_indicator_weight_and_position branch from e6c8a1a to 1e0d21e Compare May 3, 2023 11:24
@flutter-dashboard
Copy link

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

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

Changes reported for pull request #125883 at sha 1e0d21e8bef46a8695259f6f3f62bd192a6a2b6e

@TahaTesser TahaTesser requested a review from Piinks May 3, 2023 12:30
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

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

LGTM!

@Piinks
Copy link
Contributor

Piinks commented May 4, 2023

Goldens are approved. Thank you!

@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label May 4, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 5, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 6, 2023
tarrinneal pushed a commit to flutter/packages that referenced this pull request May 6, 2023
Manual roll Flutter from 0b65723 to 43ac23b (30 revisions)

Manual roll requested by [email protected]

flutter/flutter@0b65723...43ac23b

2023-05-05 [email protected] targets/web.dart - fix typo
(flutter/flutter#126114)
2023-05-05 [email protected] Roll Flutter Engine from
6b467df16e11 to 758cbadfac1f (2 revisions) (flutter/flutter#126175)
2023-05-05 [email protected] add tests for
dominant bottom sheet in scaffold (flutter/flutter#124472)
2023-05-05 [email protected] Added
CupertinoDatepicker monthYear mode (flutter#93508)
(flutter/flutter#125603)
2023-05-05 [email protected] Add
`Switch.trackOutlineWidth` property (flutter/flutter#125848)
2023-05-05 [email protected] Rename
iosdeeplinksettings to iosuniversallinksettings (flutter/flutter#126173)
2023-05-05 [email protected] Roll Flutter Engine from
f3efe11f4449 to 6b467df16e11 (3 revisions) (flutter/flutter#126174)
2023-05-05 [email protected] improvement :
removed required kotlin dependency (flutter/flutter#125002)
2023-05-05 [email protected] Fix incorrect assert hint in flutter.groovy
(flutter/flutter#125283)
2023-05-05 [email protected] Update
.cirrus.yml (flutter/flutter#126166)
2023-05-05 [email protected] tool: replace top-level
functions with enum properties (flutter/flutter#126167)
2023-05-05 [email protected] Use direct dart API from
`dart:ui_web` rather than JS shim. (flutter/flutter#123443)
2023-05-05 [email protected] Add sample
code for SliverAppBar (flutter/flutter#125785)
2023-05-05 [email protected] Roll Flutter Engine from
cef0e9d1a94f to f3efe11f4449 (3 revisions) (flutter/flutter#126163)
2023-05-05 [email protected] Add a ReorderableListView example with
cards + cleanup existing tests (flutter/flutter#126155)
2023-05-05 [email protected] Roll Packages from
6bd59cd to a0f8fd8 (4 revisions) (flutter/flutter#126161)
2023-05-05 [email protected] Bring back
the failing build_test's (flutter/flutter#126014)
2023-05-05 [email protected] [web] Use plain platform views in
benchmarks (flutter/flutter#126080)
2023-05-05 [email protected] Fix Material 3 tab indicator weight and
position (flutter/flutter#125883)
2023-05-05 [email protected] Roll Flutter Engine from
b0f53e7751ad to cef0e9d1a94f (1 revision) (flutter/flutter#126150)
2023-05-05 [email protected] Roll Flutter Engine from
6f26066144fb to b0f53e7751ad (2 revisions) (flutter/flutter#126148)
2023-05-05 [email protected] Roll Flutter Engine from
764991e046c6 to 6f26066144fb (1 revision) (flutter/flutter#126141)
2023-05-05 [email protected] Roll Flutter Engine from
e7cd29153aa9 to 764991e046c6 (1 revision) (flutter/flutter#126137)
2023-05-05 [email protected] Roll Flutter Engine from
a885ed472eea to e7cd29153aa9 (1 revision) (flutter/flutter#126135)
2023-05-05 [email protected] Roll Flutter Engine from
c97a0deccbc1 to a885ed472eea (1 revision) (flutter/flutter#126129)
2023-05-05 [email protected] [NavigationDrawer] adds padding
property in NavigationDrawer Widget (flutter/flutter#123961)
2023-05-05 [email protected] Roll Flutter Engine from
269ce2deebeb to c97a0deccbc1 (1 revision) (flutter/flutter#126124)
2023-05-05 [email protected] Roll Flutter Engine from
4d5070672859 to 269ce2deebeb (16 revisions) (flutter/flutter#126115)
2023-05-05 [email protected] Minor fixes found while working on blankcanvas
(flutter/flutter#125751)
2023-05-05 [email protected] tool-web: use ProcessUtil.run
to invoke child processes (flutter/flutter#126109)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC
[email protected],[email protected],[email protected] on the
revert to ensure that a human
is aware of the problem.

To file a bug in Packages:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
...
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
)

Manual roll Flutter from 0b65723 to 43ac23b (30 revisions)

Manual roll requested by [email protected]

flutter/flutter@0b65723...43ac23b

2023-05-05 [email protected] targets/web.dart - fix typo
(flutter/flutter#126114)
2023-05-05 [email protected] Roll Flutter Engine from
6b467df16e11 to 758cbadfac1f (2 revisions) (flutter/flutter#126175)
2023-05-05 [email protected] add tests for
dominant bottom sheet in scaffold (flutter/flutter#124472)
2023-05-05 [email protected] Added
CupertinoDatepicker monthYear mode (flutter#93508)
(flutter/flutter#125603)
2023-05-05 [email protected] Add
`Switch.trackOutlineWidth` property (flutter/flutter#125848)
2023-05-05 [email protected] Rename
iosdeeplinksettings to iosuniversallinksettings (flutter/flutter#126173)
2023-05-05 [email protected] Roll Flutter Engine from
f3efe11f4449 to 6b467df16e11 (3 revisions) (flutter/flutter#126174)
2023-05-05 [email protected] improvement :
removed required kotlin dependency (flutter/flutter#125002)
2023-05-05 [email protected] Fix incorrect assert hint in flutter.groovy
(flutter/flutter#125283)
2023-05-05 [email protected] Update
.cirrus.yml (flutter/flutter#126166)
2023-05-05 [email protected] tool: replace top-level
functions with enum properties (flutter/flutter#126167)
2023-05-05 [email protected] Use direct dart API from
`dart:ui_web` rather than JS shim. (flutter/flutter#123443)
2023-05-05 [email protected] Add sample
code for SliverAppBar (flutter/flutter#125785)
2023-05-05 [email protected] Roll Flutter Engine from
cef0e9d1a94f to f3efe11f4449 (3 revisions) (flutter/flutter#126163)
2023-05-05 [email protected] Add a ReorderableListView example with
cards + cleanup existing tests (flutter/flutter#126155)
2023-05-05 [email protected] Roll Packages from
6bd59cd to a0f8fd8 (4 revisions) (flutter/flutter#126161)
2023-05-05 [email protected] Bring back
the failing build_test's (flutter/flutter#126014)
2023-05-05 [email protected] [web] Use plain platform views in
benchmarks (flutter/flutter#126080)
2023-05-05 [email protected] Fix Material 3 tab indicator weight and
position (flutter/flutter#125883)
2023-05-05 [email protected] Roll Flutter Engine from
b0f53e7751ad to cef0e9d1a94f (1 revision) (flutter/flutter#126150)
2023-05-05 [email protected] Roll Flutter Engine from
6f26066144fb to b0f53e7751ad (2 revisions) (flutter/flutter#126148)
2023-05-05 [email protected] Roll Flutter Engine from
764991e046c6 to 6f26066144fb (1 revision) (flutter/flutter#126141)
2023-05-05 [email protected] Roll Flutter Engine from
e7cd29153aa9 to 764991e046c6 (1 revision) (flutter/flutter#126137)
2023-05-05 [email protected] Roll Flutter Engine from
a885ed472eea to e7cd29153aa9 (1 revision) (flutter/flutter#126135)
2023-05-05 [email protected] Roll Flutter Engine from
c97a0deccbc1 to a885ed472eea (1 revision) (flutter/flutter#126129)
2023-05-05 [email protected] [NavigationDrawer] adds padding
property in NavigationDrawer Widget (flutter/flutter#123961)
2023-05-05 [email protected] Roll Flutter Engine from
269ce2deebeb to c97a0deccbc1 (1 revision) (flutter/flutter#126124)
2023-05-05 [email protected] Roll Flutter Engine from
4d5070672859 to 269ce2deebeb (16 revisions) (flutter/flutter#126115)
2023-05-05 [email protected] Minor fixes found while working on blankcanvas
(flutter/flutter#125751)
2023-05-05 [email protected] tool-web: use ProcessUtil.run
to invoke child processes (flutter/flutter#126109)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC
[email protected],[email protected],[email protected] on the
revert to ensure that a human
is aware of the problem.

To file a bug in Packages:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: fidelity Matching the OEM platforms better a: quality A truly polished experience autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. c: new feature Nothing broken; request for a new capability f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. will affect goldens Changes to golden files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TabBar height and vertical text alignment don't match spec

2 participants