Skip to content

Conversation

@mhelmstadter
Copy link
Contributor

No description provided.

BrowserModule,
BrowserAnimationsModule,
FeCommonModule,
MatToolbarModule
Copy link
Member

Choose a reason for hiding this comment

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

Is MatToolBarModule required here? These can get out of controller quickly so we want to limit how much is included in the master scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope I missed that one, I'll remove it

exports: [
LoginComponent, TextBoxComponent, CheckBoxComponent]
declarations: [TextBoxComponent, LoginComponent, CheckBoxComponent, ToolbarComponent, RaisedButtonComponent, BasicButtonComponent],
exports: [LoginComponent, TextBoxComponent, CheckBoxComponent, ToolbarComponent, RaisedButtonComponent, MatFormFieldModule],
Copy link
Member

Choose a reason for hiding this comment

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

Just like the above comment, are all these exports needed?

})
export class CheckBoxComponent implements OnInit {

label = 'Remember my credentials'
Copy link
Member

Choose a reason for hiding this comment

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

This label should be dynamic and passed as @input(). Before merging these all presentation components need to be completed or at least in a reusable state.

Copy link
Member

@Castyr Castyr left a comment

Choose a reason for hiding this comment

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

Presentation components need to be refactored with @input and @output decorators in order to make them reusable. The Smart component Login is not complete, before merging to master these will need to be completed.

@@ -0,0 +1,3 @@
<mat-toolbar color="primary">
<span>{{title}}</span>
</mat-toolbar> No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

No newline at end of file. There are plug-ins for Visual Studio Code that will do this automatically for you.

src/index.html Outdated
<meta name="viewport" content="width=device-width, initial-scale=1">
<link rel="icon" type="image/x-icon" href="favicon.ico">
<link href="https://fonts.googleapis.com/icon?family=Material+Icons" rel="stylesheet">
<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Roboto:300,400,500,700,400italic">
Copy link
Member

Choose a reason for hiding this comment

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

Are you using all these font types? Only include the ones you are using because they add to the load time of the page.

<p>
DRIVER-CHECKBOX
</p>
<mat-checkbox [color]="color">{{label}}</mat-checkbox>
Copy link
Member

Choose a reason for hiding this comment

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

checkbox needs to store a model that can be retrieved and have an event emitter for changes.

@@ -0,0 +1,3 @@
<button mat-icon-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 will need a click event emitter

@@ -0,0 +1,17 @@
<mat-card>
<mat-card-title>
SidenavBasic
Copy link
Member

Choose a reason for hiding this comment

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

is this being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I wasn't sure if I should add the sidebar files in case we use them later, but I can get rid of them.

<input type="text" [id]="id" [placeholder]="placeholder" [value]="value" (click)="click($event)" />
<div class="form-template">
<mat-form-field [color]="warn">
<input matInput placeholder={{placeholder}} [type]="hide ? 'password' : 'text'">
Copy link
Member

Choose a reason for hiding this comment

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

mutliple event emitters need to be exposed here, onkeyup and etc... Also, there needs to be a way to describe a model that can be passed from a parent.

@mhelmstadter mhelmstadter merged commit febc2d6 into master Nov 10, 2017
@mhelmstadter mhelmstadter deleted the CreateLoginScreen branch November 10, 2017 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants