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

Button tokens was missing iOS File and had a typo in Default file #3238

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

gremlin529
Copy link
Contributor

Platforms Impacted

  • iOS
  • macOS
  • win32 (Office)
  • windows
  • android

Description of changes

(a summary of the changes made, often organized by file)

Verification

ran local tests and test code

Before After
Screenshot or description before this change Screenshot or description with this change

Pull request checklist

This PR has considered (when applicable):

  • Automated Tests
  • Documentation and examples
  • Keyboard Accessibility
  • Voiceover
  • Internationalization and Right-to-left Layouts

The color tokens file for iOS was missing, and the default one had a couple of types (icon: instead of iconColor:)
@gremlin529 gremlin529 requested a review from a team as a code owner November 16, 2023 16:23

export const defaultButtonColorTokens: TokenSettings<ButtonTokens, Theme> = (t: Theme) =>
({
backgroundColor: t.colors.buttonBackground,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a copy of the base ButtinColorTokens file? If so this file isn't necessary, it should pick up the base version if no override has been specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me check that, I actually noticed this was missing and the bugs in the other one several weeks ago then forgot to go through with the check in so I'm resurecting it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you do need iOS tokens, we should use Fluent tokens to specify the styling. The current colors listed here are older colors that aren't used in the Fluent2 system.

@rurikoaraki
Copy link
Collaborator

Please add a description of your changes.

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.

2 participants