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

refactor(multiple): switch to non-deprecated MDC list styles #22504

Merged
merged 1 commit into from
Jun 22, 2021

Conversation

crisbeto
Copy link
Member

Switches the components in mdc-autocomplete, mdc-core, mdc-menu and mdc-select to the non-deprecated MDC list styles.

Note for reviewer: merging this is currently blocked, because it depends on one change that isn't in the latest canary version. Once the change is released, I'll bump the version and unblock merging. The code can still be reviewed.

@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround blocked This issue is blocked by some external factor, such as a prerequisite PR merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release labels Apr 18, 2021
@crisbeto crisbeto requested a review from mmalerba April 18, 2021 07:08
@crisbeto crisbeto requested a review from a team as a code owner April 18, 2021 07:08
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 18, 2021
@crisbeto
Copy link
Member Author

crisbeto commented Apr 18, 2021

Caretaker note: in the process of migrating the styles for mdc-menu I noticed one place which didn't have the correct markup, causing the menu items to have bold text. I've corrected it here, but it may cause some screenshot diff failures. If it's problematic, I can split the markup fix into a separate PR.

@crisbeto crisbeto force-pushed the mdc-list-evolution branch 3 times, most recently from 9fbe441 to 8fb3c36 Compare April 18, 2021 08:23
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM. One minor comment/question.

@mmalerba
Copy link
Contributor

@crisbeto is this available in canary now?

@crisbeto
Copy link
Member Author

It's not available in canary yet. It's waiting for material-components/material-components-web#7021.

@crisbeto
Copy link
Member Author

Unblocking this since the necessary changes are in the canary branch now.

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed blocked This issue is blocked by some external factor, such as a prerequisite PR labels May 13, 2021
@crisbeto crisbeto force-pushed the mdc-list-evolution branch 3 times, most recently from 97faee6 to 89dc94a Compare June 3, 2021 16:09
@crisbeto
Copy link
Member Author

crisbeto commented Jun 3, 2021

FYI, I had to lock us to a specific canary version, because having the caret in the version caused yarn to install an older canary version of MDC for some reason. I've locked us to the current latest canary.

crisbeto added a commit to crisbeto/material2 that referenced this pull request Jun 4, 2021
Our MDC version requirement currently uses a caret with the intention of allowing any version after the specified canary. This isn't actually how npm works and it can end up installing an earlier version. The version isn't technically valid semver, causing the package manager to sort the versions alphabetically and to pick the last one. This behavior threw me off when rebasing angular#22504 yesterday, because it ended up installing a version that was too old.

These changes lock us to a specific version.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jun 7, 2021
Our MDC version requirement currently uses a caret with the intention of allowing any version after the specified canary. This isn't actually how npm works and it can end up installing an earlier version. The version isn't technically valid semver, causing the package manager to sort the versions alphabetically and to pick the last one. This behavior threw me off when rebasing angular#22504 yesterday, because it ended up installing a version that was too old.

These changes lock us to a specific version.
andrewseguin pushed a commit that referenced this pull request Jun 8, 2021
Our MDC version requirement currently uses a caret with the intention of allowing any version after the specified canary. This isn't actually how npm works and it can end up installing an earlier version. The version isn't technically valid semver, causing the package manager to sort the versions alphabetically and to pick the last one. This behavior threw me off when rebasing #22504 yesterday, because it ended up installing a version that was too old.

These changes lock us to a specific version.
@crisbeto crisbeto force-pushed the mdc-list-evolution branch 2 times, most recently from 803ef9a to 24a3d6c Compare June 21, 2021 18:46
Switches the components in `mdc-autocomplete`, `mdc-core`, `mdc-menu` and `mdc-select` to the non-deprecated MDC list styles.
@mmalerba mmalerba merged commit f960a7a into angular:master Jun 22, 2021
mmalerba pushed a commit that referenced this pull request Jun 22, 2021
Switches the components in `mdc-autocomplete`, `mdc-core`, `mdc-menu` and `mdc-select` to the non-deprecated MDC list styles.

(cherry picked from commit f960a7a)
mmalerba pushed a commit that referenced this pull request Jun 22, 2021
Switches the components in `mdc-autocomplete`, `mdc-core`, `mdc-menu` and `mdc-select` to the non-deprecated MDC list styles.

(cherry picked from commit f960a7a)
mmalerba added a commit that referenced this pull request Jun 25, 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 Jul 23, 2021
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 cla: yes PR author has agreed to Google's Contributor License Agreement merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note 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.

None yet

3 participants