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

feat(badge): add badge component #7483

Merged
merged 20 commits into from
Feb 13, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions src/demo-app/badge/badge-demo.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
<div class="badge-demo">
Copy link
Member

Choose a reason for hiding this comment

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

It would be good if the demo had a button to toggle badge visibility to see the animation


<h3>Icons</h3>
<mat-badge [content]="badgeContent">
<mat-icon color="accent">home</mat-icon>
</mat-badge>

<mat-badge content="1" overlap="false">
<mat-icon color="primary">home</mat-icon>
</mat-badge>

<br />
<br />
<br />

<h3>Buttons</h3>
<mat-badge content="22">
<button mat-raised-button>
<mat-icon color="primary">home</mat-icon>
</button>
</mat-badge>

<mat-badge content="1">
<button mat-raised-button>
Email
</button>
</mat-badge>

<br />
<br />
<br />

<h3>Text</h3>
<mat-badge [overlap]="false" content="22">
Email
</mat-badge>

<mat-badge overlap="false" content="John">
Email
</mat-badge>

<br />
Copy link
Member

Choose a reason for hiding this comment

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

Use a div with a margin instead of br-s?

<br />
<br />

<h3>Text Directions</h3>
<mat-badge overlap="false" direction="above before" content="22" color="primary">
Email
</mat-badge>

<mat-badge overlap="false" direction="above after" content="22" color="accent">
Email
</mat-badge>

<mat-badge overlap="false" direction="below before" content="22" color="warn">
Email
</mat-badge>

<mat-badge overlap="false" direction="below after" content="22">
Email
</mat-badge>

<br />
<br />
<br />

<h3>Icon Directions</h3>
<mat-badge direction="above before" content="22">
<mat-icon color="primary">home</mat-icon>
</mat-badge>

<mat-badge direction="above after" content="22">
<mat-icon color="primary">home</mat-icon>
</mat-badge>

<mat-badge direction="below before" content="22">
<mat-icon color="primary">home</mat-icon>
</mat-badge>

<mat-badge direction="below after" content="22">
<mat-icon color="primary">home</mat-icon>
</mat-badge>

</div>
Empty file.
11 changes: 11 additions & 0 deletions src/demo-app/badge/badge-demo.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import {Component} from '@angular/core';

@Component({
moduleId: module.id,
selector: 'badge-demo',
templateUrl: 'badge-demo.html',
styleUrls: ['badge-demo.css'],
})
export class BadgeDemo {
badgeContent = '1';
}
1 change: 1 addition & 0 deletions src/demo-app/demo-app/demo-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export class DemoApp {
dark = false;
navItems = [
{name: 'Autocomplete', route: '/autocomplete'},
{name: 'Badge', route: '/badge'},
Copy link
Member

Choose a reason for hiding this comment

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

Routing to the badge in the demo app fails

Uncaught (in promise): Error: Cannot match any routes. URL Segment: 'badge'

{name: 'Button Toggle', route: '/button-toggle'},
{name: 'Button', route: '/button'},
{name: 'Card', route: '/card'},
Expand Down
2 changes: 2 additions & 0 deletions src/demo-app/demo-app/demo-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {TooltipDemo} from '../tooltip/tooltip-demo';
import {TypographyDemo} from '../typography/typography-demo';
import {DemoApp, Home} from './demo-app';
import {DEMO_APP_ROUTES} from './routes';
import {BadgeDemo} from '../badge/badge-demo';

@NgModule({
imports: [
Expand All @@ -59,6 +60,7 @@ import {DEMO_APP_ROUTES} from './routes';
],
declarations: [
AutocompleteDemo,
BadgeDemo,
BaselineDemo,
ButtonDemo,
ButtonToggleDemo,
Expand Down
2 changes: 2 additions & 0 deletions src/demo-app/demo-app/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,13 @@ import {ToolbarDemo} from '../toolbar/toolbar-demo';
import {TooltipDemo} from '../tooltip/tooltip-demo';
import {TypographyDemo} from '../typography/typography-demo';
import {DemoApp, Home} from './demo-app';
import {BadgeDemo} from '../badge/badge-demo';

export const DEMO_APP_ROUTES: Routes = [
{path: '', component: DemoApp, children: [
{path: '', component: Home},
{path: 'autocomplete', component: AutocompleteDemo},
{path: 'badge', component: BadgeDemo},
{path: 'baseline', component: BaselineDemo},
{path: 'button', component: ButtonDemo},
{path: 'button-toggle', component: ButtonToggleDemo},
Expand Down
2 changes: 2 additions & 0 deletions src/demo-app/demo-material-module.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {NgModule} from '@angular/core';
import {
MatAutocompleteModule,
MatBadgeModule,
MatButtonModule,
MatButtonToggleModule,
MatCardModule,
Expand Down Expand Up @@ -46,6 +47,7 @@ import {PortalModule} from '@angular/cdk/portal';
@NgModule({
exports: [
MatAutocompleteModule,
MatBadgeModule,
MatButtonModule,
MatButtonToggleModule,
MatCardModule,
Expand Down
1 change: 1 addition & 0 deletions src/demo-app/system-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ System.config({
'@angular/material/tabs': 'dist/packages/material/tabs/index.js',
'@angular/material/toolbar': 'dist/packages/material/toolbar/index.js',
'@angular/material/tooltip': 'dist/packages/material/tooltip/index.js',
'@angular/material/badge': 'dist/packages/material/badge/index.js',
},
packages: {
// Thirdparty barrels.
Expand Down
42 changes: 42 additions & 0 deletions src/lib/badge/_badge-theme.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
@import '../core/theming/palette';
@import '../core/theming/theming';
@import '../core/typography/typography-utils';

Copy link
Member

Choose a reason for hiding this comment

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

Add comment here that explains why the badge theme contains all of the styles for the badge instead of just color and typography

$mat-badge-font-size: 12px;
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 probably be coming in through the typography configuration. @crisbeto thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that would be ideal, but I don't see any breakpoint that maps nicely. @amcdnl is this font size somewhere in the spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@crisbeto - There is no spec for badges.

Copy link
Member

Choose a reason for hiding this comment

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

Would it work to just use the font-size from caption?

$mat-badge-font-weight: 600;

@mixin _mat-badge-color($palette) {
background: mat-color($palette);
color: mat-color($palette, default-contrast);
}

@mixin mat-badge-theme($theme) {
$accent: map-get($theme, accent);
$warn: map-get($theme, warn);
$primary: map-get($theme, primary);

.mat-badge {
.mat-badge-content {
Copy link
Member

Choose a reason for hiding this comment

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

Looks like .mat-badge-content doesn't need to be nested

Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong indentation?

@include _mat-badge-color($primary);
}

&.mat-accent {
.mat-badge-content {
Copy link
Member

Choose a reason for hiding this comment

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

This (and other styles below) looks like it has more nesting than necessary

E.g., could could just have

.mat-badge-content {
  &.mat-badge-primary { ... }
  &.mat-badge-accent { ... }
  &.mat-badge-warn { ... }
}

(without needing the parent .mat-badge)

Copy link
Contributor Author

@amcdnl amcdnl Jan 19, 2018

Choose a reason for hiding this comment

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

Would it work to just use the font-size from caption?

The size needs to be absolute for the sizing of the bubble.

@include _mat-toolbar-color($accent);
}
}

&.mat-warn {
.mat-badge-content {
@include _mat-toolbar-color($warn);
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

There's an extra closing } here (masked by the extra indentation in the mixin block)


@mixin mat-badge-typography($config) {
.mat-badge-content {
font-weight: $mat-badge-font-weight;
font-size: $mat-badge-font-size;
Copy link
Member

Choose a reason for hiding this comment

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

Size and weight should come from the typography config, no? cc @crisbeto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sizes different based on the small|med|large so needed to make them independent.

}
}
22 changes: 22 additions & 0 deletions src/lib/badge/badge-module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {NgModule} from '@angular/core';
import {MatCommonModule} from '@angular/material/core';
import {MatBadge} from './badge';


@NgModule({
imports: [MatCommonModule],
exports: [
MatBadge,
MatCommonModule,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to export MatCommonModule here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. thanks

],
declarations: [MatBadge],
})
export class MatBadgeModule {}
5 changes: 5 additions & 0 deletions src/lib/badge/badge.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<ng-content></ng-content>
<span
class="mat-badge-content"
[innerHTML]="content">
Copy link
Member

Choose a reason for hiding this comment

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

This looks dangerous. Is there any reason people would want to put anything besides plain text in a badge?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in latest.

</span>
35 changes: 35 additions & 0 deletions src/lib/badge/badge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
Badges are small status descriptors for UI elements. A badge consists of a small circle,
typically containing a number or other short set of characters, that appears in proximity to another object.
Copy link
Member

Choose a reason for hiding this comment

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

Wrap at 100 columns (here and below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unclear what you are asking here, can you elaborate?


### Customizing badge position
By default, the badge will be placed `above after`. The direction can be changed by defining
the attribute `direction` follow by `above|below` and `before|after`.

```html
<mat-badge direction="above after" content="22">
<mat-icon>home</mat-icon>
</mat-badge>
```

The overlap of the badge in relation to its inner contents can also be defined
using the `overlap` tag. Typically, you want the badge to overlap an icon and not
a text phrase. By default it will overlap.

```html
<mat-badge overlap="false" content="22">
Email
</mat-badge>
```

### Theming
Badges can be colored in terms of the current theme using the `color` property to set the
background color to `primary`, `accent`, or `warn`.

```html
<mat-badge color="accent" content="22">
<mat-icon>home</mat-icon>
</mat-badge>
```
Copy link
Member

Choose a reason for hiding this comment

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

Docs should also mention customizing the size and hiding


### Accessibility
Badges should be given a meaningful label via `aria-label` or `aria-labelledby` attributes.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure whether this is correct. Shouldn't the badge target have an aria-describedby instead @jelbourn?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to reconsider how we want to do this with latest api changes.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is right. This section should talk about how the badges use aria-describedby to augment the host element.

64 changes: 64 additions & 0 deletions src/lib/badge/badge.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
$mat-badge-size: 22px !default;

.mat-badge {
position: relative;
display: inline-block;

&.mat-badge-above {
Copy link
Member

Choose a reason for hiding this comment

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

This nesting seems like it's bumping the specificity unnecessarily. Consider moving it out instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this quite a bit, you might want to re-review.

.mat-badge-content {
top: -$mat-badge-size / 2;
}
}

&.mat-badge-below {
.mat-badge-content {
bottom: -$mat-badge-size / 2;
}
}

&.mat-badge-before {
margin-left: $mat-badge-size;
.mat-badge-content {
left: -$mat-badge-size;
}
}

&.mat-badge-after {
margin-right: $mat-badge-size;
.mat-badge-content {
right: -$mat-badge-size;
}
}

&.mat-badge-overlap {
&.mat-badge-before {
margin-left: $mat-badge-size / 2;
.mat-badge-content {
left: -$mat-badge-size / 2;
}
}

&.mat-badge-after {
margin-right: $mat-badge-size / 2;
.mat-badge-content {
right: -$mat-badge-size / 2;
}
}
}

.mat-badge-content {
display: flex;
flex-direction: row;
flex-wrap: wrap;
justify-content: center;
Copy link
Member

Choose a reason for hiding this comment

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

Does this really need flexbox? It can be done simpler with text-align and line-height.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It now supports font icons and svgs, so its needed now.

align-content: center;
align-items: center;
position: absolute;
width: $mat-badge-size;
height: $mat-badge-size;
border-radius: 50%;
text-overflow: ellipsis;
white-space: nowrap;
overflow: hidden;
}
}
Loading