Skip to content

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?

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