Skip to content

refactor(material/divider): switch to tokens API #27478

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

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

behzadmehrabi
Copy link
Contributor

No description provided.

@behzadmehrabi behzadmehrabi force-pushed the divider-tokens branch 7 times, most recently from cc3d271 to 8b83294 Compare July 19, 2023 23:16
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution, but I wanted to note that for the other components we try to do the token conversions within the team, because landing the changes internally often requires a lot of debugging and tweaking the CSS. The divider is simple enough that we can go with this PR though.

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto crisbeto self-assigned this Jul 21, 2023
@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Jul 21, 2023
@behzadmehrabi
Copy link
Contributor Author

behzadmehrabi commented Jul 21, 2023

Thank you for the contribution, but I wanted to note that for the other components we try to do the token conversions within the team, because landing the changes internally often requires a lot of debugging and tweaking the CSS. The divider is simple enough that we can go with this PR though.

Your welcome, I appreciate your feedback :)...I have 2 questions out of curiosity, if you don't mind.

  1. How do you decide whether a component tokens should be defined in m2/mat or m2/mdc? why something like radio is defined in both? cuz I was a bit confused about where to put divider tokens.
  2. is there any specs that you look into for components that are not MDC based when defining the tokens?

@crisbeto
Copy link
Member

  1. The tokens in the MDC directory are the ones that go through MDC's theming API. The rest are either components that MDC didn't have equivalents for or some tokens we added ourselves that we were missing.
  2. There's an internal set of tokens that we sometimes try to consult. Otherwise we make up the tokens ourselves while trying to keep with the terminology from the spec (e.g. using "tracking" instead of "letter spacing").

@crisbeto crisbeto merged commit cc02ea8 into angular:main Jul 21, 2023
crisbeto pushed a commit that referenced this pull request Jul 21, 2023
AnthonyFr75 pushed a commit to AnthonyFr75/components that referenced this pull request Jul 31, 2023
stephenrca pushed a commit to stephenrca/components that referenced this pull request Aug 2, 2023
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants