Skip to content

Fix title color of license detail page#63249

Merged
fluttergithubbot merged 2 commits intoflutter:masterfrom
mono0926:fix-license-page-title-color
Aug 19, 2020
Merged

Fix title color of license detail page#63249
fluttergithubbot merged 2 commits intoflutter:masterfrom
mono0926:fix-license-page-title-color

Conversation

@mono0926
Copy link
Contributor

@mono0926 mono0926 commented Aug 8, 2020 •
edited
Loading

Description

Fix #63239.

Related Issues

Tests

I didn't add any tests because existing about_test.dart doesn't contain color related test, but I'll write test if needed.

I added the following tests:

Titles of _PackageLicensePage use correct text style in flutter/test/material/about_test.dart.

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.

@flutter-dashboard
Copy link

flutter-dashboard bot commented Aug 8, 2020

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.

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

1 similar comment
@flutter-dashboard
Copy link

flutter-dashboard bot commented Aug 8, 2020

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.

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 8, 2020
@mono0926 mono0926 mentioned this pull request Aug 8, 2020
Merged
12 tasks
@gspencergoog
Copy link
Contributor

gspencergoog commented Aug 14, 2020

Thanks @mono0926, this change makes sense. Can you please add a test that verifies that if there is an appbar text theme, it uses that instead of the primary text theme?

@mono0926 mono0926 force-pushed the fix-license-page-title-color branch from 23899ae to ba9d7f1 Compare August 15, 2020 03:04
@mono0926
Copy link
Contributor Author

mono0926 commented Aug 15, 2020

@gspencergoog

Thanks for the feedback.

Can you please add a test that verifies that if there is an appbar text theme, it uses that instead of the primary text theme?

I've added tests, so could you review it? 🙏

@gspencergoog
Copy link
Contributor

gspencergoog commented Aug 18, 2020

@mono0926 I've re-run the failing web tests a few times, and they keep failing. I'm quite sure that doesn't have to do with your change, so can you try syncing to master/HEAD and pushing an update? I think you're just synced to a bad ref.

@mono0926 mono0926 force-pushed the fix-license-page-title-color branch 3 times, most recently from d1ef97d to 6de7e89 Compare August 19, 2020 03:00
@mono0926 mono0926 force-pushed the fix-license-page-title-color branch from 6de7e89 to 265c68e Compare August 19, 2020 05:44
@mono0926
Copy link
Contributor Author

mono0926 commented Aug 19, 2020

@gspencergoog

so can you try syncing to master/HEAD and pushing an update?

I did and all checks have passed ✅

gspencergoog
gspencergoog approved these changes Aug 19, 2020
View reviewed changes
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

Will submit when the tree is green

@fluttergithubbot fluttergithubbot merged commit 606abc9 into flutter:master Aug 19, 2020
@mono0926 mono0926 deleted the fix-license-page-title-color branch August 19, 2020 21:04
smadey pushed a commit to smadey/flutter that referenced this pull request Aug 27, 2020
@mono0926 @smadey
Fix title color of license detail page (flutter#63249)
b3bfce7
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@mono0926 @mingwandroid
Fix title color of license detail page (flutter#63249)
bfea01c
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@gspencergoog gspencergoog gspencergoog 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.

The title of license detail page isn't visible depending on the Theme configuration

4 participants

@mono0926 @gspencergoog @googlebot @fluttergithubbot