Skip to content

docs(button): enhance button docs and examples #8536

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

docs(button): enhance button docs and examples #8536

wants to merge 1 commit into from

Conversation

EladBezalel
Copy link
Member

changed the overview example to
image

and change examples section to
image

showing use cases

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 18, 2017
@EladBezalel EladBezalel mentioned this pull request Nov 18, 2017
Open
amcdnl
amcdnl reviewed Nov 19, 2017
View reviewed changes
src/material-examples/button-toolbar/button-toolbar-example.css Outdated
@@ -0,0 +1,3 @@
.spacer {
flex: 1;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should end w/ new line

amcdnl
amcdnl reviewed Nov 19, 2017
View reviewed changes
src/material-examples/button-toolbar/button-toolbar-example.html Outdated
<button mat-icon-button>
<mat-icon aria-label="Example icon-button with a more vertical icon">more_vert</mat-icon>
</button>
</mat-toolbar>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should end w/ new line

amcdnl
amcdnl reviewed Nov 19, 2017
View reviewed changes
src/material-examples/button-toolbar/button-toolbar-example.ts
templateUrl: 'button-toolbar-example.html',
styleUrls: ['button-toolbar-example.css'],
})
export class ButtonToolbarExample {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should end w/ new line

amcdnl
amcdnl previously requested changes Nov 19, 2017
View reviewed changes
Copy link
Contributor

@amcdnl amcdnl left a comment

Choose a reason for hiding this comment

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

Overall looks good but need to go back through and ensure files end w/ new line.

@EladBezalel
Copy link
Member Author

fixed

jelbourn
jelbourn reviewed Nov 21, 2017
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

FYI the docs site also needs to be updated once this is merged:

https://github.com/angular/material.angular.io/blob/master/src/app/shared/documentation-items/documentation-items.ts

(someday we'll get around to auto-generating this)

src/material-examples/button-card/button-card-example.html Outdated
<mat-card-title>Card with action buttons</mat-card-title>
</mat-card-header>
<mat-card-actions align="end">
<button mat-button>CANCEL</button>
Copy link
Member

Choose a reason for hiding this comment

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

Use normal text case for buttons, the docs site capitalizes them via CSS. If screen-readers encounter ALL CAPS, it reads it as "A. L. L. C. ..."

(here and elsewhere)

src/material-examples/button-overview/button-overview-example.css Outdated
@@ -1 +1,16 @@
/** No CSS for this example */
.button-row {
Copy link
Member

Choose a reason for hiding this comment

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

Prefix all example styles with .example- (here and elsewhere)

This makes it super obvious which styles are from the example vs. the docs site (docs-) vs. material

src/material-examples/button-card/button-card-example.html Outdated
</mat-card-header>
<mat-card-actions align="end">
<button mat-button>CANCEL</button>
<button mat-button color="primary">ACCEPT</button>
Copy link
Member

Choose a reason for hiding this comment

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

These should have an aria-label along the lines of "Example button, no action"

(here and elsewhere, where the label can be expanded with what the button is an example of)

src/material-examples/button-overview/button-overview-example.html Outdated
<h3>Icon Buttons</h3>
<div class="button-row">
<button mat-icon-button>
<mat-icon aria-label="Example icon-button with a heart icon">favorite</mat-icon>
Copy link
Member

Choose a reason for hiding this comment

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

https://material.angular.io/components/icon/overview#accessibility

In cases like this the aria-label should go on the interactive element (the button or anchor)

src/material-examples/example-module.ts Outdated
@@ -17,10 +17,11 @@ import {AutocompleteDisplayExample} from './autocomplete-display/autocomplete-di
import {AutocompleteFilterExample} from './autocomplete-filter/autocomplete-filter-example';
import {AutocompleteOverviewExample} from './autocomplete-overview/autocomplete-overview-example';
import {AutocompleteSimpleExample} from './autocomplete-simple/autocomplete-simple-example';
import {ButtonCardExample} from './button-card/button-card-example';
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if you knew this but this file is auto-generated

(I'm not sure why it's in source control... @amcdnl?)

@EladBezalel
Copy link
Member Author

angular/material.angular.io#330 here's the pr for the material.angular.io website

@EladBezalel
Copy link
Member Author

@jelbourn it seems like once i removed example-module.ts changes it's breaking..

@jelbourn
Copy link
Member

The changes to example-module are necessary, but you shouldn't need to make them by hand. The file also probably shouldn't need to be in source control (a matter for another PR)

@jelbourn
Copy link
Member

@EladBezalel could you rebase this PR?

@josephperrott
Copy link
Member

@EladBezalel could you rebase this PR (again)?

@josephperrott
Copy link
Member

Closing as we are going on ~6 months of inactvity on this PR. Feel free to open up a new PR with these changes if you would like to get them into the library.

@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 Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jelbourn jelbourn jelbourn left review comments

@amcdnl amcdnl amcdnl left review comments

@tinayuangao tinayuangao Awaiting requested review from tinayuangao

Assignees
No one assigned
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@EladBezalel @jelbourn @josephperrott @amcdnl @googlebot