Skip to content

docs(material/slide-toggle): update changes from legacy #25825

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
Oct 21, 2022

Conversation

zarend
Copy link
Contributor

@zarend zarend commented Oct 17, 2022

Update migration guide for MDC Slide Toggle to give more detail on form validation. Remove form validation from slide-toggle example as it does not seem to be appropriate for switch role. Update slide toggle overview documentation to reflect changes for the MDC-based slide toggle.

@zarend zarend added docs This issue is related to documentation merge safe area: material/slide-toggle labels Oct 17, 2022
@zarend zarend requested a review from mmalerba October 17, 2022 13:46
@angular-robot angular-robot bot added the area: docs Related to the documentation label Oct 17, 2022

<mat-slide-toggle name="slideToggle" required ngModel>
<mat-slide-toggle name="slideToggle" ngModel="[(formToggle)]">
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what doesn't look right about it? required attribute doesn't work, since this is a button role="switch". I also added the binding to formToggle, so that it can alert the value of the toggle when submitting the form.

Copy link
Member

Choose a reason for hiding this comment

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

The syntax on the ngModel binding is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohh, that's not the right syntax 🤦 🍌 📦. I corrected the syntax, thanks.

Update migration guide for MDC Slide Toggle to give more detail on form
validation. Remove form validation from slide-toggle example as it does
not seem to be appropriate for switch role. Update slide toggle overview
documentation to reflect changes for the MDC-based slide toggle.
@zarend zarend force-pushed the slide-toggle-docs branch from e219c45 to af30754 Compare October 21, 2022 11:07
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

@zarend zarend added action: merge The PR is ready for merge by the caretaker target: rc This PR is targeted for the next release-candidate labels Oct 21, 2022
@zarend zarend merged commit 7456712 into angular:main Oct 21, 2022
zarend added a commit that referenced this pull request Oct 21, 2022
Update migration guide for MDC Slide Toggle to give more detail on form
validation. Remove form validation from slide-toggle example as it does
not seem to be appropriate for switch role. Update slide toggle overview
documentation to reflect changes for the MDC-based slide toggle.

(cherry picked from commit 7456712)
@zarend zarend deleted the slide-toggle-docs branch October 21, 2022 11:25
@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 Nov 21, 2022
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 area: docs Related to the documentation area: material/slide-toggle docs This issue is related to documentation target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants