Skip to content

Conversation

@paldepind
Copy link
Contributor

This PR updates the Cupertino system colors figure with the missing colors mint, cyan and brown as mentioned in flutter/flutter#118970 and as added in the PR flutter/flutter#118971.

I also changed the ordering of the colors to match the order they have in the Apple HIG. This order sorts the colors based on their position in the color wheel (I think) and is much more visually pleasing.

With the three extra colors, the figure did not fit in the viewport available for the diagrams. I henced increased its size a bit in order to fit the figure.

Note the that this PR depends on flutter/flutter#118971 as it uses the new colors added there. Hence it should only be merged after that.

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 the [Flutter Style Guide] recently, and have followed its advice.
  • 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.

@paldepind
Copy link
Contributor Author

The CI failing is expected as the PR only compiles with the changes from flutter/flutter#118971.

@MitchellGoodwin
Copy link

This PR is going to need to wait until the updated colors are in stable before it's merged.

@Hixie
Copy link
Contributor

Hixie commented Apr 18, 2023

It's been about three months since our last stable so the next stable should be soon!

@MitchellGoodwin
Copy link

@paldepind Hello 👋. The updated colors are in stable now, so would you mind rebasing to restart the tests?

@MitchellGoodwin
Copy link

@paldepind the base branch was changed from master_archived to main. Would you be able to rebase on top of main?

@domesticmouse domesticmouse changed the base branch from master_archived to main May 31, 2023 21:45
@domesticmouse
Copy link

I've updated the target branch and brought the PR up to date. Going off the previous CI results the code may still require formatting.

Copy link

@MitchellGoodwin MitchellGoodwin left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for putting this together.

@paldepind
Copy link
Contributor Author

Thanks @domesticmouse 🙂

@domesticmouse domesticmouse merged commit 83c5787 into flutter:main Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants