Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update hintColor value for Material #65036

Merged
merged 1 commit into from Sep 2, 2020
Merged

Update hintColor value for Material #65036

merged 1 commit into from Sep 2, 2020

Conversation

JuYeong0413
Copy link
Contributor

@JuYeong0413 JuYeong0413 commented Sep 1, 2020

Description

Updated hintColor value for Material.

Both Dark mode documentation and Light mode documentation states that

Medium-emphasis text and hint text have opacities of 60%

This pull request updates hintColor value in dark mode to Colors.white60(corresponds to 60% opacity).
For light mode, updates value to Colors.black.withOpacity(0.6) since there's no constant for black60.

For golden file changes:

input_decorator.outline_icon_label.ltr.png
input_decorator outline_icon_label ltr
input_decorator.outline_icon_label.rtl.png
input_decorator outline_icon_label rtl

Related Issues

Fixes #55329

Tests

None. Only color values changed.
If I need to add a test for this, please let me know. :)

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.
  • Yes, this is a breaking change.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Sep 1, 2020
@flutter-dashboard
Copy link

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.

@skia-gold
Copy link

Gold has detected about 6 untriaged digest(s) on patchset 1.
View them at https://flutter-gold.skia.org/cl/github/65036

@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 #65036 at sha 35f95e0

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Sep 1, 2020
@JuYeong0413
Copy link
Contributor Author

JuYeong0413 commented Sep 1, 2020

Hi @Piinks, this is my first time making golden file changes, so I might have not fully understood about Golden File Test.
(Sorry for pinging, looks like you're busy these days!)

I ran flutter test locally with --update-goldens flag and got updated images.
From the wiki, it seems like I should add goldens in my PR description, but I didn't get it. Does this mean that I should I open a pull request in flutter/goldens repository?
The README in the goldens repository says:

The flutter/goldens repository has been deprecated from its use for the flutter/flutter repository’s golden file testing.

so I'm a bit confused about what should I do next with updated images...

@Piinks
Copy link
Contributor

Piinks commented Sep 1, 2020

Hey @JuYeong0413! You can just include the image in your PR description above. :)
The flutter-dashboard comment above provided a link to the dashboard where you can see the images that were generated by the tests run here. If they look as you expect we can approve them and they'll be checked in on the dashboard.
Let me know if the images on the dashboard look right, here is the link again:

Click here to view and triage

@JuYeong0413
Copy link
Contributor Author

Ah, now I've got it. Thanks a lot @Piinks!
I've added images in the description. Please take a look when you have time. :)

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.

Validated this change is covered by golden file tests.
Thank you fort he contribution @JuYeong0413 ! LGTM!

@fluttergithubbot fluttergithubbot merged commit 0b4dad6 into flutter:master Sep 2, 2020
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
@JuYeong0413 JuYeong0413 deleted the material-hintcolor branch September 11, 2020 14:05
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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.

Need to update hintColor value for Material
5 participants