Skip to content

Revert "Add support for Material 3 Divider and VerticalDivider" - #112471

Merged
auto-submit[bot] merged 1 commit into
masterfrom
revert-112378-m3_divider
Sep 27, 2022
Merged

auto-submit[bot] merged 1 commit into
masterfrom
revert-112378-m3_divider

Conversation

@CaseyHillers

@CaseyHillers CaseyHillers commented Sep 27, 2022 •

Copy link
Copy Markdown
Contributor

Reverts #112378

@TahaTesser this broke a few Google tests.

It seems like it's not pulling the correct color from the theme.

    final Color? color = this.color ?? defaults.color;

Googlers, see b/249195768

@flutter-dashboard flutter-dashboard Bot added d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos documentation p: material_ui material_ui package in flutter/packages framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Sep 27, 2022
@CaseyHillers CaseyHillers added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 27, 2022
@auto-submit
auto-submit Bot merged commit bd5c347 into master Sep 27, 2022
@auto-submit
auto-submit Bot deleted the revert-112378-m3_divider branch September 27, 2022 15:16
@TahaTesser

Copy link
Copy Markdown
Contributor

Thanks! I'll take a look.

@TahaTesser

TahaTesser commented Sep 28, 2022 •

Copy link
Copy Markdown
Contributor

@darrenaustin
Can you please check google tests? In the tests or visually, there is no difference in default M2 colors.

M2 template doesn't set any color and leaves to createBorderSide to resolve just like before this PR.

master PR

@darrenaustin

Copy link
Copy Markdown
Contributor

@darrenaustin Can you please check google tests? In the tests or visually, there is no difference in default M2 colors.

So a couple of unrelated issues. The tests that failed were golden tests that were using a divider with useMaterial3: true, but with default values. This PR changes those defaults, so the tests failed. I have worked with the team to update the app to use ThemeData.dividerTheme to ensure their dividers won't change when this lands.

The second issue was one I didn't catch in review that @CaseyHillers brought up above: we weren't using the DividerTheme.of in the color calculation (at least not in the M3 case).

I took the liberty of taking your PR and changing some of the color logic to fix this, as well as adding more of the properties to the generated theme data so that all the properties followed the same pattern. Hope you don't mind. The new PR is: #112655.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants