Skip to content

Conversation

@SimonVillage
Copy link
Contributor

Based on the sketch file from https://developer.apple.com/design/resources/ the tab bar label has a font weigh for medium / 500.

Original iOS tab bar

IMG_3739

Top: Cupertino without font weight, bottom: changed font weight to medium

Group 1

fixes #90108

@flutter-dashboard flutter-dashboard bot added the f: cupertino flutter/packages/flutter/cupertino repository label Sep 15, 2021
@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.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 15, 2021
@google-cla
Copy link

google-cla bot commented Sep 15, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Sep 15, 2021
@SimonVillage
Copy link
Contributor Author

@googlebot I signed it!

@google-cla google-cla bot added cla: yes and removed cla: no labels Sep 15, 2021
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.

Hi @SimonHausdorf welcome! And thank you for the contribution. This will need a test to verify the change, and to prevent it from regressing in the future. Can you add a test?

@Hixie
Copy link
Contributor

Hixie commented Dec 15, 2021

@SimonHausdorf Is this a PR you are still interested in working on?

@SimonVillage
Copy link
Contributor Author

@Hixie maybe someone else could contribute to the test part? I am not exactly sure what the test should contain or how it would look like.

@SimonVillage
Copy link
Contributor Author

@Piinks I added a test for the Cupertino text styles. Let me know if there should be any addition.

#92197 removed the workaround to fix the wrong font weight for the label.

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.

Hey @SimonHausdorf thanks for coming back to this and adding a test! Can you rebase with master to update your branch? That should resolve the failing checks.

@SimonVillage
Copy link
Contributor Author

@Piinks some tests are still failing but I assume they are not related to the PR?

@Piinks
Copy link
Contributor

Piinks commented Feb 18, 2022

I see, thanks for pinging. In this case, you will need to rebase in order to resolve those test failures. Enough time has passed for this PR that merging causes issues for some tests (#97595).

@SimonVillage SimonVillage requested a review from Piinks February 19, 2022 08:29
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.

Flutter_LGTM

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@Piinks
Copy link
Contributor

Piinks commented Feb 25, 2022

Oh whoops, I forgot this needs 2 reviews, I have reached out to the team for another reviewer

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 27, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 28, 2022
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CupertinoTabBar / BottomNavigationBarItem wrong label font weight

5 participants