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(stark-ui): implement the button's theme #517

Merged
merged 1 commit into from
Jul 18, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion packages/stark-ui/assets/styles/_colors.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ $mat-light-theme-foreground: $foregrounds;

@if variable-exists(stark-base-theme) {
$base-theme: map-merge($base-theme, $stark-base-theme);

@if map-has-key($stark-base-theme, backgrounds) {
$mat-light-theme-background: map-merge($backgrounds, map-get($stark-base-theme, backgrounds));
}
Expand Down
4 changes: 2 additions & 2 deletions packages/stark-ui/assets/styles/_typography.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ $typography-config: mat-typography-config(
$display-1: mat-typography-level-create(map-get($typography-theme, display-1)),
$headline: mat-typography-level-create(map-get($typography-theme, headline)),
$title: mat-typography-level-create(map-get($typography-theme, title)),
$subheading-2: mat-typography-level-create(map-get($typography-theme, subheading-2)), /*Menu, List, Option*/
$subheading-1: mat-typography-level-create(map-get($typography-theme, subheading-1)),
$subheading-2: mat-typography-level-create(map-get($typography-theme, subheading-2)),
/*Menu, List, Option*/ $subheading-1: mat-typography-level-create(map-get($typography-theme, subheading-1)),
$body-2: mat-typography-level-create(map-get($typography-theme, body-2)),
$body-1: mat-typography-level-create(map-get($typography-theme, body-1)),
$caption: mat-typography-level-create(map-get($typography-theme, caption)),
Expand Down
67 changes: 34 additions & 33 deletions packages/stark-ui/assets/styles/_variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,20 @@ $accent-palette: (
);

$warning-palette: (
50: #ffccbc,
100: #ffcdd2,
200: #ef9a9a,
300: #e57373,
400: #ef5350,
500: #f44336,
600: #e53935,
700: #d32f2f,
800: #c62828,
900: #b71c1c,
A100: #ff8a80,
A200: #ff5252,
A400: #ff1744,
A700: #d50000,
50: #fff3e0,
100: #ffe0b2,
200: #ffcc80,
300: #ffb74d,
400: #ffa726,
500: #ff9800,
600: #fb8c00,
700: #f57c00,
800: #ef6c00,
900: #e65100,
A100: #ffd180,
A200: #ffab40,
A400: #ff9100,
A700: #ff6d00,
contrast: (
50: $dark-primary-text,
100: $dark-primary-text,
Expand All @@ -93,9 +93,9 @@ $warning-palette: (
800: $light-primary-text,
900: $light-primary-text,
A100: $dark-primary-text,
A200: $light-primary-text,
A400: $light-primary-text,
A700: $light-primary-text
A200: $dark-primary-text,
A400: $dark-primary-text,
A700: black
)
);

Expand Down Expand Up @@ -136,25 +136,26 @@ $foregrounds: (
$base-theme: (
primary-palette: mat-palette($primary-palette),
accent-palette: mat-palette($accent-palette),
warn-palette: mat-palette($mat-red)
warn-palette: mat-palette($warning-palette)
);

$button-theme: (
border-radius: $border-radius,
font-size: 14px,
font-weight: 500,
line-height: 36px,
padding: 0 16px,
small-font-size: 12px,
small-font-weight: 500,
small-line-height: 26px,
small-min-width: 50px,
small-padding: 0 5px,
large-font-size: 30px,
large-font-weight: 500,
large-line-height: 52px,
large-min-width: 88px,
large-padding: 0 20px
alert: (
color: #d32f2f,
contrast: $light-primary-text
),
alt: (
color: #323232,
contrast: $light-primary-text
),
neutral: (
color: #bfbfbf,
contrast: $light-primary-text
),
success: (
color: #4caf50,
contrast: $light-primary-text
)
);

$card-theme: (
Expand Down
89 changes: 64 additions & 25 deletions packages/stark-ui/assets/themes/_button-theme.scss
Original file line number Diff line number Diff line change
@@ -1,38 +1,77 @@
@mixin stark-button-color($color) {
color: $color;
.mat-button-focus-overlay {
background-color: rgba($color: $color, $alpha: 0.12);
@mixin stark-button-color($color, $contrast) {
&.mat-button,
&.mat-icon-button,
&.mat-stroked-button {
color: $color;
.mat-button-focus-overlay {
background-color: rgba($color: $color, $alpha: 0.12);
}
.mat-ripple-element {
background-color: rgba($color: $color, $alpha: 0.1);
}
}
.mat-ripple-element {
background-color: rgba($color: $color, $alpha: 0.1);

&.mat-flat-button,
&.mat-raised-button,
&.mat-fab,
&.mat-mini-fab {
color: $contrast;
background-color: $color;
.mat-ripple-element {
background-color: rgba($color: $contrast, $alpha: 0.1);
}
}

.mat-icon-button {
.mat-ripple-element {
background-color: rgba($color: $color, $alpha: 0.2);
}
}
}

@if variable-exists(stark-button-theme) {
$button-theme: map-merge($button-theme, $stark-button-theme);
}

button[mat-raised-button],
a[mat-raised-button] {
border-radius: map-get($button-theme, border-radius);
/*padding: map-get($button-theme, padding);
line-height: map-get($button-theme, line-height);*/
.mat-success {
@include stark-button-color(map-get(map-get($button-theme, success), color), map-get(map-get($button-theme, success), contrast));
}

.mat-alert {
@include stark-button-color(map-get(map-get($button-theme, alert), color), map-get(map-get($button-theme, alert), contrast));
}

.mat-alt {
@include stark-button-color(map-get(map-get($button-theme, alt), color), map-get(map-get($button-theme, alt), contrast));
}

.mat-neutral {
@include stark-button-color(map-get(map-get($button-theme, neutral), color), map-get(map-get($button-theme, neutral), contrast));
}

button[mat-raised-button].stark-small-button,
a[mat-raised-button].stark-small-button {
font-size: map-get($button-theme, small-font-size);
font-weight: map-get($button-theme, small-font-weight);
line-height: map-get($button-theme, small-line-height);
min-width: map-get($button-theme, small-min-width);
padding: map-get($button-theme, small-padding);
.mat-white {
@include stark-button-color(#fff, $dark-primary-text);
}

.mat-icon-button .mat-icon,
.mat-mini-fab .mat-icon {
&.stark-small-icon {
line-height: 18px;
height: 18px;
width: 18px;
svg {
height: 18px;
width: 18px;
}
}
}

button[mat-raised-button].stark-large-button,
a[mat-raised-button].stark-large-button {
font-size: map-get($button-theme, large-font-size);
font-weight: map-get($button-theme, large-font-weight);
line-height: map-get($button-theme, large-line-height);
min-width: map-get($button-theme, large-min-width);
padding: map-get($button-theme, large-padding);
.mat-button,
.mat-icon-button,
.mat-stroked-button,
.mat-flat-button,
.mat-raised-button,
.mat-fab,
.mat-mini-fab {
text-transform: uppercase;
}
Original file line number Diff line number Diff line change
@@ -1,37 +1,47 @@
.stark-action-bar {
.mat-icon-button {
@include stark-button-color(mat-color(map-get($base-theme, primary-palette)));
}
&.stark-action-bar-full {
background-color: rgba(0, 0, 0, 0.85);
color: rgba(255, 255, 255, 0.8);
.mat-icon-button {
@include stark-button-color(rgba($color: #fff, $alpha: 0.8));
}
@media #{$tablet-query} {
.stark-action-bar {
.action-label {
&.primary {
color: mat-color(map-get($base-theme, primary-palette));
}
&.warn {
color: mat-color(map-get($base-theme, warn-palette));
}
&.accent {
color: mat-color(map-get($base-theme, accent-palette));
}
&.success {
color: map-get(map-get($button-theme, success), color);
}
&.alert {
color: map-get(map-get($button-theme, alert), color);
}
&.alt {
color: map-get(map-get($button-theme, alt), color);
}
&.neutral {
color: map-get(map-get($button-theme, neutral), color);
}
&.white {
color: #fff;
}
&.disabled {
color: rgba($color: #fff, $alpha: 0.25);
color: map-get($mat-light-theme-foreground, disabled-text);
}
}
}
}

@media #{$tablet-query} {
.stark-action-bar {
color: $md-primary;
&.stark-action-bar-full {
background: none;
color: mat-color(map-get($base-theme, primary-palette));
.mat-icon-button:not([disabled]) {
@include stark-button-color(mat-color(map-get($base-theme, primary-palette)));
}
[disabled].mat-icon-button {
@include stark-button-color(map-get($mat-light-theme-foreground, disabled-button));
}
.action-label {
&.disabled {
color: map-get($mat-light-theme-foreground, disabled-button);
}
@media #{$mobile-only-query} {
.stark-action-bar-full {
background-color: rgba(0, 0, 0, 0.85);
.mat-icon-button {
@include stark-button-color(rgba($color: #fff, $alpha: 0.8), $light-primary-text);
}
.action-label {
color: rgba(255, 255, 255, 0.8);
&.disabled {
color: rgba($color: #fff, $alpha: 0.25);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@
align-items: center;
flex-wrap: wrap;
}
.mat-icon-button {
.mat-icon svg {
height: 20px;
width: 20px;
}
}

&.stark-action-bar-full {
bottom: 0;
left: 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,27 @@
<div *ngFor="let action of actionBarConfig.actions; trackBy: trackAction" [id]="actionBarId + '-' + action.id"
(click)="onClick(action, $event)" class="stark-action-bar--action">
<ng-container *ngIf="action.isVisible !== false">
<button [ngClass]="action.className" [matTooltip]="action.labelSwitchFunction ? action.labelActivated : action.label" [disabled]="!action.isEnabled" mat-icon-button>
<mat-icon starkSvgViewBox [svgIcon]="action.iconSwitchFunction ? action.iconActivated : action.icon"></mat-icon>
<button [color]="action.buttonColor ? action.buttonColor:'primary'" [ngClass]="action.className" [matTooltip]="action.labelSwitchFunction ? action.labelActivated : action.label" [disabled]="!action.isEnabled" mat-icon-button>
<mat-icon starkSvgViewBox [svgIcon]="action.iconSwitchFunction ? action.iconActivated : action.icon" class="stark-small-icon"></mat-icon>
</button>
<label class="action-label" [ngClass]="{'disabled': !action.isEnabled}" *ngIf="isExtended" translate>{{ action.label }}</label>
<label [class]="'action-label '+ (action.buttonColor?action.buttonColor:'primary')" [ngClass]="{'disabled': !action.isEnabled}" *ngIf="isExtended"> <span translate>{{ action.label }}</span></label>
</ng-container>
</div>
</div>

<div *ngIf="mode === 'full' || alternativeActions" class="alt-actions">
<button class="extend-action-bar" mat-icon-button *ngIf="mode === 'full'" (click)="toggleExtendedActionBar()">
<mat-icon starkSvgViewBox svgIcon="dots-horizontal"></mat-icon>
<button class="extend-action-bar" color="primary" mat-icon-button *ngIf="mode === 'full'" (click)="toggleExtendedActionBar()">
Copy link
Member

Choose a reason for hiding this comment

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

Why force setting primary color?
Maybe should we provide a new property in the interface to set a color? Which is an optional string and, if it is not set, we can set primary by default. What do you think about this? 😊

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 is a feature coming from Angular Material, nothing added on our side.

https://material.angular.io/components/button/examples

Copy link
Collaborator

@christophercr christophercr Jul 17, 2018

Choose a reason for hiding this comment

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

I think what @SuperITMan is talking about is the fact that we "enforce" the primary color on the action buttons. I'm also not sure if we should enforce such color, what if the developer wants a different color? How can he change the color without affecting the primary color of other buttons?

As I mentioned the other day, I think the safest approach will be not to use the Angular Material color directive in the Stark packages but only in the client apps like the showcase and the starter.

Copy link
Contributor Author

@catlabs catlabs Jul 17, 2018

Choose a reason for hiding this comment

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

If we want a closest integration with Angular Material, I think we should keep the directive that they provide. Now I have extend the 3 default colors to add the one we were using before. Then the end user could use Angular Material button with our custom colors, I think that provide the most flexibility.

Imagine if we give up on the color attr and they write this:
<button color="primary" class="stark-alert" mat-raised-button>Primary or Alert?</button>
Then this button have 2 styles defined, so one should override the other

With this notation:
<button color="alert" mat-raised-button>Alert</button>
There is no confusion and we stay inline with Angular Material

Copy link
Contributor Author

@catlabs catlabs Jul 17, 2018

Choose a reason for hiding this comment

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

If we really want to override the color we can do it through a mixin that I have created, which use actually the angular material mixin for buttons.

The code for end user will be:

.my-custom-button {
  @include stark-button-color(#fff, #222); //(color, text-color)
}

Copy link
Member

Choose a reason for hiding this comment

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

@catlabs What do you think if we add a property in StarkAction ?
Now, we have something like this:

export interface StarkAction {
  // ...
  isEnabled: boolean;
  label: string;
  // ...
}

But we could have something else like:

export interface StarkAction {
  // ...
  isEnabled: boolean;
  label: string;
  buttonColor: string;    //    <-------
  // ...
}

Then, in the component, we could have this:

<button color="action.buttonColor ? action.buttonColor : 'alert'" mat-raised-button>Alert</button>

What do you think about this ?

We continue to use Angular Material as it should be but we give more abilities to choose the color of the button.
I don't think we should use @mixin function to change the color of the button...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well yeah that's an idea, we talked about it with @christophercr actually, I ll do that and repush the change

Copy link
Collaborator

Choose a reason for hiding this comment

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

Mmm, but then we should think about what are the possible colors we would accept; any? just primary, accent and alert?

If we accept any, in case a new color is given, then it means that the developer should also "create" that color with the Angular Material mixins which at the end willl be the same effort that the one @catlabs mentioned above in order to override the color.

So, I don't know if this approach of having a buttonColor in the interface will make things easier or not

Copy link
Member

Choose a reason for hiding this comment

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

I think you're right Christopher 😊

We could do so

{
  //... 
  buttonColor: "primary" | "alert" | "accent";
} 

What do you think about this?
And if they want more flexibility, they will have to play with @mixin function 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well yeah that's the idea, they can either enter one of the seven colors we provide (primary, warn,accent,success,alert,alt,neutral,white) or a custom one, then they will have to play with the mixin

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this "color" should also be set via the buttonColor 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.

Well this button are not set via any Input property, it is the generic extend button. I don't think we should add an Input for this, it will make the api more complex.

<mat-icon starkSvgViewBox svgIcon="dots-horizontal" class="stark-small-icon"></mat-icon>
</button>
<button class="open-alt-actions" mat-icon-button *ngIf="alternativeActions" [matMenuTriggerFor]="menu">
<mat-icon starkSvgViewBox svgIcon="dots-vertical"></mat-icon>
<button class="open-alt-actions" color="primary" mat-icon-button *ngIf="alternativeActions" [matMenuTriggerFor]="menu">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, I think this "color" should also be set via the buttonColor 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.

Same remark than above

<mat-icon starkSvgViewBox svgIcon="dots-vertical" class="stark-small-icon"></mat-icon>
</button>
<mat-menu #menu="matMenu" xPosition="before">
<div *ngFor="let action of alternativeActions; trackBy: trackAction" mat-menu-item [id]="actionBarId + '-alt-' + action.id" [disabled]="!action.isEnabled"
(click)="onClick(action, $event)">
<ng-container *ngIf="action.isVisible !== false">
<div [ngClass]="action.className">
<mat-icon starkSvgViewBox [svgIcon]="action.icon"></mat-icon>
<mat-icon starkSvgViewBox [svgIcon]="action.icon" class="stark-small-icon"></mat-icon>
<span translate>{{ action.label }}</span>
</div>
</ng-container>
Expand Down
Loading