Skip to content

feat(stepper): Add initial styles to stepper based on Material guidelines - #6242

Merged
g1shin merged 5 commits into
angular:stepperfrom
g1shin:css
Aug 6, 2017
Merged

feat(stepper): Add initial styles to stepper based on Material guidelines#6242
g1shin merged 5 commits into
angular:stepperfrom
g1shin:css

Conversation

@g1shin

@g1shing1shin commented Aug 3, 2017

Copy link
Copy Markdown

screen shot 2017-08-04 at 3 48 26 pm

screen shot 2017-08-04 at 3 48 33 pm

@googlebotgooglebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 3, 2017
Comment threadsrc/lib/stepper/_stepper-theme.scss Outdated
}

.mat-stepper-index {
background: mat-color($primary);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: background-color

Comment threadsrc/lib/stepper/_stepper-theme.scss Outdated
}

.mat-stepper-horizontal, .mat-stepper-vertical {
background: mat-color($background, 'card');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

background-color

Comment threadsrc/lib/stepper/stepper.scss Outdated

:host {
display: block;
padding-left: 24px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

create scss var for this

Comment threadsrc/lib/stepper/stepper.scss Outdated
overflow: hidden;
text-overflow: ellipsis;
flex-shrink: 1;
min-width: 50px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scss var

Comment threadsrc/lib/stepper/stepper.scss Outdated
.mat-horizontal-stepper-header-container {
white-space: nowrap;
display: flex;
padding-right: 24px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scss var

Comment threadsrc/lib/stepper/stepper.scss Outdated
.connector-line {
border: 0;
height: 1px;
border-top: 1px solid #bdbdbd;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

border-top-color should be part of theme file

Comment threadsrc/lib/stepper/stepper.scss Outdated

.vertical-content-container {
content: '';
border-left: 1px solid #bdbdbd;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scss vars, border-left-color in theme file

Comment threadsrc/lib/stepper/stepper.scss Outdated
}

.mat-vertical-stepper-content {
margin-left: 24px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scss var

Comment threadsrc/lib/stepper/stepper.scss Outdated
}

&[aria-expanded='true'] {
padding-bottom: 48px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scss vars

Comment threadsrc/lib/stepper/stepper.scss Outdated
}

&:first-child {
padding-top: 24px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scss var

@g1shin

Copy link
Copy Markdown
Author

Ready for review again 😀

@mmalerbammalerba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you using pixel margins to vertically center things? This is usually a bad idea because its very brittle. I can show you how to use some of the flexbox alignment properties to do this more easily

[attr.aria-expanded]="selectedIndex == i">
<ng-container [ngTemplateOutlet]="step.content"></ng-container>
<div class="vertical-content-container">
<!--<div *ngIf="!isLast" class="vertical-connector-line"></div>-->

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove

Comment threadsrc/lib/stepper/stepper.scss Outdated
.mat-stepper-content[aria-expanded='false'] {
display: none;
$mat-horizontal-stepper-header-height: 72px;
$mat-vertical-stepper-header-height: 24px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are any of these values related to each other or conceptually the same thing? for example I see a lot of 24px

Comment threadsrc/lib/stepper/stepper.scss Outdated
.mat-horizontal-stepper-header {
display: inline-flex;
line-height: $mat-horizontal-stepper-header-height;
flex-grow: 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can use shorthand for flex-grow and flex-shrink: flex: 0 1 auto

display: inline-flex;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does this work now? if not leave a TODO to investigate

Comment threadsrc/lib/stepper/stepper.scss Outdated
margin-right: $mat-horizontal-stepper-index-margin-right;
margin-top: $mat-horizontal-stepper-index-martin-top;
display: inline-block;
flex-shrink: 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

flex: none

Comment threadsrc/lib/stepper/stepper.scss Outdated
border-top-style: solid;
margin-top: $mat-connector-line-margin-top;
width: 5%;
flex-grow: 1;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

flex: auto

Comment threadsrc/lib/stepper/stepper.scss Outdated
flex-grow: 1;
flex-shrink: 1;
margin-left: $mat-connector-line-margin;
margin-right: $mat-connector-line-margin;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can use margin shorthand: margin: <top> <right> 0 <left>

Comment threadsrc/lib/stepper/stepper.scss Outdated
}

.vertical-content-container {
content: '';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

?

Comment threadsrc/lib/stepper/stepper.scss Outdated
border-top-width: $mat-connector-line-width;
border-top-style: solid;
margin-top: $mat-connector-line-margin-top;
width: 5%;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scss var

Comment threadsrc/lib/stepper/stepper.scss Outdated
content: '';
border-left-width: $mat-connector-line-width;
border-left-style: solid;
margin: $mat-connector-line-margin 0 $mat-connector-line-margin 12px;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scss var for 12px

@g1shin

Copy link
Copy Markdown
Author

Changes made; ready for review.

@ocarreteromocarreterom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SCSS vars should be !default.

@g1shin

Copy link
Copy Markdown
Author

Changes have been made regarding managing margins, and I deployed the modified demo onto Firebase app. 👍

@mmalerbammalerba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd like to further refine the CSS in the future, but for now it looks good

@g1shin
g1shin merged commit b0f11d6 into angular:stepperAug 6, 2017
@g1shin
g1shin deleted the css branch August 6, 2017 18:23
g1shin pushed a commit that referenced this pull request Aug 14, 2017
…ines (#6242)
* Add initial styles to stepper based on Material guidelines
* Fix flex-shrink and min-width
* Changes made based on review
* Fix alignment
* Margin modifications
g1shin pushed a commit that referenced this pull request Aug 16, 2017
…ines (#6242)
* Add initial styles to stepper based on Material guidelines
* Fix flex-shrink and min-width
* Changes made based on review
* Fix alignment
* Margin modifications
g1shin pushed a commit to g1shin/material2 that referenced this pull request Aug 22, 2017
…ines (angular#6242)
* Add initial styles to stepper based on Material guidelines
* Fix flex-shrink and min-width
* Changes made based on review
* Fix alignment
* Margin modifications
g1shin pushed a commit to g1shin/material2 that referenced this pull request Aug 22, 2017
…ines (angular#6242)
* Add initial styles to stepper based on Material guidelines
* Fix flex-shrink and min-width
* Changes made based on review
* Fix alignment
* Margin modifications
g1shin pushed a commit to g1shin/material2 that referenced this pull request Aug 22, 2017
…ines (angular#6242)
* Add initial styles to stepper based on Material guidelines
* Fix flex-shrink and min-width
* Changes made based on review
* Fix alignment
* Margin modifications
g1shin pushed a commit that referenced this pull request Aug 23, 2017
…ines (#6242)
* Add initial styles to stepper based on Material guidelines
* Fix flex-shrink and min-width
* Changes made based on review
* Fix alignment
* Margin modifications
mmalerba pushed a commit that referenced this pull request Aug 23, 2017
…tepper branch. (#5742)
* Prototyping
* Further work
* Further prototyping
* Further prototyping
* Further work
* Adding event emitters
* Adding "selectedIndex" attribute to stepper and working on TemplateOulet.
* Prototyping
* Further work
* Further prototyping
* Further prototyping
* Further work
* Adding event emitters
* Template rendering and selectIndex control done.
* Work in progress for accessibility
* Added functionalities based on the tentative API doc.
* Refactor code for cdk-stepper and cdk-step
* Add support for templated label
* Added support for keyboard events and focus changes for accessibility.
* Updated vertical stepper + added comments
* Fix package-lock.json
* Fix indention
* Changes made based on the review
* Changes based on review - event properties, selectors, SPACE support, etc. + demo
* Add select() for step component + refactor to avoid circular dependency + support cycling using arrow keys
* API change based on review
* Minor code clean up based on review.
* Several name changes, etc based on review
* Add to compatibility mode list and refactor to avoid circular dependency
feat(stepper): Create stepper button directives to enable adding buttons to stepper (#5951)
* Create stepper button directives to enable adding buttons to stepper
* Changes made based on review
* Minor changes with click handlers
Build changes
feat(stepper): Add initial styles to stepper based on Material guidelines (#6242)
* Add initial styles to stepper based on Material guidelines
* Fix flex-shrink and min-width
* Changes made based on review
* Fix alignment
* Margin modifications
feat(stepper): Add support for linear stepper (#6116)
* Add form controls and custom error state matcher
* Modify form controls for stepper-demo and add custom validator
* Move custom step validation function so that users can simply import and use
* Implement @input() stepControl for each step
* Add linear attribute to stepper
* Add enabling/disabling linear state of demo
feat(stepper): Add animation to stepper (#6361)
* Add animation
* Implement Angular animation
* Clean up unnecessary code
* Generalize animation so that vertical and horizontal steppers can use the same function
Rebase onto upstream/master
feat(stepper): Add unit tests for stepper (#6428)
* Add unit tests for stepper
* Changes made based on review
* More changes based on review
feat(stepper): Add support for linear stepper #2 - each step as its own form. (#6117)
* Add form control - consider each step as its own form group
* Comment edits
* Add 'valid' to MdStep for form validation
* Add [stepControl] to each step based on merging
* Changes based on review
Fix focus logic and CSS changes (#6507)
feat(stepper): Add documentation for stepper (#6533)
* Documentation for stepper
* Revision based on review + add accessibility section
feat(stepper): Support additional properties for step (#6509)
* Additional properties for step
* Unit tests
* Code changes based on review + test name changes
* Refactor code for shared functionality between vertical and horizontal stepper
* Refactor md-step-header and md-step-content + optional step change
* Simplify code based on review
* Changes to step-header based on review
* Minor changes
Fix host style and demo page (#6592)
Revert package.json and package-lock.json
Changes made along with BUILD changes in google3
Add typography mixin
Changes to address aot compiler failures
fix rtl bugs
g1shin pushed a commit to g1shin/material2 that referenced this pull request Aug 31, 2017
…tepper branch. (angular#5742)
* Prototyping
* Further work
* Further prototyping
* Further prototyping
* Further work
* Adding event emitters
* Adding "selectedIndex" attribute to stepper and working on TemplateOulet.
* Prototyping
* Further work
* Further prototyping
* Further prototyping
* Further work
* Adding event emitters
* Template rendering and selectIndex control done.
* Work in progress for accessibility
* Added functionalities based on the tentative API doc.
* Refactor code for cdk-stepper and cdk-step
* Add support for templated label
* Added support for keyboard events and focus changes for accessibility.
* Updated vertical stepper + added comments
* Fix package-lock.json
* Fix indention
* Changes made based on the review
* Changes based on review - event properties, selectors, SPACE support, etc. + demo
* Add select() for step component + refactor to avoid circular dependency + support cycling using arrow keys
* API change based on review
* Minor code clean up based on review.
* Several name changes, etc based on review
* Add to compatibility mode list and refactor to avoid circular dependency
feat(stepper): Create stepper button directives to enable adding buttons to stepper (angular#5951)
* Create stepper button directives to enable adding buttons to stepper
* Changes made based on review
* Minor changes with click handlers
Build changes
feat(stepper): Add initial styles to stepper based on Material guidelines (angular#6242)
* Add initial styles to stepper based on Material guidelines
* Fix flex-shrink and min-width
* Changes made based on review
* Fix alignment
* Margin modifications
feat(stepper): Add support for linear stepper (angular#6116)
* Add form controls and custom error state matcher
* Modify form controls for stepper-demo and add custom validator
* Move custom step validation function so that users can simply import and use
* Implement @input() stepControl for each step
* Add linear attribute to stepper
* Add enabling/disabling linear state of demo
feat(stepper): Add animation to stepper (angular#6361)
* Add animation
* Implement Angular animation
* Clean up unnecessary code
* Generalize animation so that vertical and horizontal steppers can use the same function
Rebase onto upstream/master
feat(stepper): Add unit tests for stepper (angular#6428)
* Add unit tests for stepper
* Changes made based on review
* More changes based on review
feat(stepper): Add support for linear stepper angular#2 - each step as its own form. (angular#6117)
* Add form control - consider each step as its own form group
* Comment edits
* Add 'valid' to MdStep for form validation
* Add [stepControl] to each step based on merging
* Changes based on review
Fix focus logic and CSS changes (angular#6507)
feat(stepper): Add documentation for stepper (angular#6533)
* Documentation for stepper
* Revision based on review + add accessibility section
feat(stepper): Support additional properties for step (angular#6509)
* Additional properties for step
* Unit tests
* Code changes based on review + test name changes
* Refactor code for shared functionality between vertical and horizontal stepper
* Refactor md-step-header and md-step-content + optional step change
* Simplify code based on review
* Changes to step-header based on review
* Minor changes
Fix host style and demo page (angular#6592)
Revert package.json and package-lock.json
Changes made along with BUILD changes in google3
Add typography mixin
Changes to address aot compiler failures
fix rtl bugs
g1shin pushed a commit to g1shin/material2 that referenced this pull request Aug 31, 2017
…tepper branch. (angular#5742)
* Prototyping
* Further work
* Further prototyping
* Further prototyping
* Further work
* Adding event emitters
* Adding "selectedIndex" attribute to stepper and working on TemplateOulet.
* Prototyping
* Further work
* Further prototyping
* Further prototyping
* Further work
* Adding event emitters
* Template rendering and selectIndex control done.
* Work in progress for accessibility
* Added functionalities based on the tentative API doc.
* Refactor code for cdk-stepper and cdk-step
* Add support for templated label
* Added support for keyboard events and focus changes for accessibility.
* Updated vertical stepper + added comments
* Fix package-lock.json
* Fix indention
* Changes made based on the review
* Changes based on review - event properties, selectors, SPACE support, etc. + demo
* Add select() for step component + refactor to avoid circular dependency + support cycling using arrow keys
* API change based on review
* Minor code clean up based on review.
* Several name changes, etc based on review
* Add to compatibility mode list and refactor to avoid circular dependency
feat(stepper): Create stepper button directives to enable adding buttons to stepper (angular#5951)
* Create stepper button directives to enable adding buttons to stepper
* Changes made based on review
* Minor changes with click handlers
Build changes
feat(stepper): Add initial styles to stepper based on Material guidelines (angular#6242)
* Add initial styles to stepper based on Material guidelines
* Fix flex-shrink and min-width
* Changes made based on review
* Fix alignment
* Margin modifications
feat(stepper): Add support for linear stepper (angular#6116)
* Add form controls and custom error state matcher
* Modify form controls for stepper-demo and add custom validator
* Move custom step validation function so that users can simply import and use
* Implement @input() stepControl for each step
* Add linear attribute to stepper
* Add enabling/disabling linear state of demo
feat(stepper): Add animation to stepper (angular#6361)
* Add animation
* Implement Angular animation
* Clean up unnecessary code
* Generalize animation so that vertical and horizontal steppers can use the same function
Rebase onto upstream/master
feat(stepper): Add unit tests for stepper (angular#6428)
* Add unit tests for stepper
* Changes made based on review
* More changes based on review
feat(stepper): Add support for linear stepper angular#2 - each step as its own form. (angular#6117)
* Add form control - consider each step as its own form group
* Comment edits
* Add 'valid' to MdStep for form validation
* Add [stepControl] to each step based on merging
* Changes based on review
Fix focus logic and CSS changes (angular#6507)
feat(stepper): Add documentation for stepper (angular#6533)
* Documentation for stepper
* Revision based on review + add accessibility section
feat(stepper): Support additional properties for step (angular#6509)
* Additional properties for step
* Unit tests
* Code changes based on review + test name changes
* Refactor code for shared functionality between vertical and horizontal stepper
* Refactor md-step-header and md-step-content + optional step change
* Simplify code based on review
* Changes to step-header based on review
* Minor changes
Fix host style and demo page (angular#6592)
Revert package.json and package-lock.json
Changes made along with BUILD changes in google3
Add typography mixin
Changes to address aot compiler failures
fix rtl bugs
@angular-automatic-lock-bot

Copy link
Copy Markdown

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-botangular-automatic-lock-botBot locked and limited conversation to collaborators Sep 6, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yesPR author has agreed to Google's Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@g1shin@ocarreterom@mmalerba@jelbourn@kara@googlebot@jwshinjwshin