Skip to content

Conversation

@mmalerba
Copy link
Contributor

No description provided.

@mmalerba mmalerba added P2 The issue is important to a large percentage of users, with a workaround G This is is related to a Google internal issue target: patch This PR is targeted for the next patch release labels Sep 21, 2021
@mmalerba mmalerba requested a review from crisbeto as a code owner September 21, 2021 22:19
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 21, 2021
Copy link
Contributor

@wagnermaciel wagnermaciel left a comment

Choose a reason for hiding this comment

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

lgtm

@wagnermaciel wagnermaciel added the action: merge The PR is ready for merge by the caretaker label Sep 22, 2021
// When the 2014 typography config is mapped to the 2018 config, the font-weight that winds up
// getting used for menu items looks slightly off. In this case we override it to a value that
// looks better.
@if (typography.private-typography-is-2014-config($config-or-theme)) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we fix this at a higher level? I think that select and autocomplete (and maybe list?) use the same set of mixins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a good point. Let's merge this PR because its smaller in scope and will unblock me for the menu migrations I'm working on, but as a follow-up I'll try to address this more holistically

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So @andrewseguin made the opposite hack for table when he was working on that migration. There the font wasn't bold enough. Looking into it more I found that if we just ignore how the levels are named, they actually map better visually if we just swap them, so that's what I've done in #23618 which allows us to remove both this hack and Andrew's 🎉

@mmalerba mmalerba added blocked This issue is blocked by some external factor, such as a prerequisite PR and removed action: merge The PR is ready for merge by the caretaker labels Sep 23, 2021
@mmalerba
Copy link
Contributor Author

Closing in favor of #23618

@mmalerba mmalerba closed this Sep 24, 2021
@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 Oct 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

blocked This issue is blocked by some external factor, such as a prerequisite PR cla: yes PR author has agreed to Google's Contributor License Agreement G This is is related to a Google internal issue P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants