Skip to content

docs: add matButtonModule to fix example #19041

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
Apr 20, 2020
Merged

docs: add matButtonModule to fix example #19041

merged 1 commit into from
Apr 20, 2020

Conversation

santoshyadavdev
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 10, 2020
@devversion
Copy link
Member

@santoshyadav198613 Is this ready for review? If not, can you please convert this to a draft.

If this is ready: I don't think any example in cdk/a11y relies on MatButton, or is the plan to add buttons here? (please explain in the commit message)

@santoshyadavdev santoshyadavdev marked this pull request as draft April 10, 2020 18:41
@santoshyadavdev
Copy link
Contributor Author

Hi @devversion ,

The UI is broken as we are using mat-button in sample code template, but not importing it.
I need help to identify lint error as well.

@santoshyadavdev santoshyadavdev marked this pull request as ready for review April 10, 2020 18:44
@devversion
Copy link
Member

@santoshyadav198613 Thanks for clarifying. Can you please share where you see the UI broken? I've looked in the docs site, and also can't see mat-button being used anywhere inside cdk/a11y examples.

@santoshyadavdev
Copy link
Contributor Author

santoshyadavdev commented Apr 10, 2020

Hi @devversion ,
a11y is rolledback i did it to test, the actual issue is https://github.com/angular/components/blob/master/src/components-examples/cdk/text-field/text-field-autofill-monitor/text-field-autofill-monitor-example.html#L12 you can see we are using mat-raised-button, but on UI
image

its normal button, also If this works will add it to a11y button as well, as I think we should follow the material design for docs.

@devversion
Copy link
Member

Ahh. I see. You moved the changes over to cdk/textfield now. That looks good. Thx!

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.

Generally LGTM. Thanks! Can you please fix the import?

@@ -1,6 +1,7 @@
import {TextFieldModule} from '@angular/cdk/text-field';
import {CommonModule} from '@angular/common';
import {NgModule} from '@angular/core';
import {MatButtonModule} from '@angular/material/Button';
Copy link
Member

Choose a reason for hiding this comment

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

This should be lower-case button.

Suggested change
import {MatButtonModule} from '@angular/material/Button';
import {MatButtonModule} from '@angular/material/button';

@santoshyadavdev
Copy link
Contributor Author

OMG, how can I do that, thanks for pointing out, looks like I was too tired :)

@devversion devversion added merge: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed lgtm action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Apr 11, 2020
@jelbourn jelbourn merged commit 1cf4048 into angular:master Apr 20, 2020
jelbourn pushed a commit that referenced this pull request Apr 20, 2020
(cherry picked from commit 1cf4048)
soro-google pushed a commit to soro-google/components that referenced this pull request Apr 24, 2020
@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 May 21, 2020
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: fix commit message When the PR is merged, rewrites/fixups of the commit messages are needed target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants