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
refactor schedule page to be an NgModule #54
Conversation
d63ee41
to
fdea8eb
Compare
they indeed just take all scss and bundle it .. |
display: inline-block; | ||
padding: 2px 4px 4px 4px; | ||
font-size: 0.8em; | ||
margin-right: 5px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very small remark: group:
margin: {
right: 5px;
bottom: 5px;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, didn't know that was possible :)
Which is weird. It's not how Angular 2 works imo. I'm going to check this and maybe create an issue in the Ionic repository. Ionic is nothing special, I don't see a reason why it wouldn't be technically possible. |
fdea8eb
to
46ec332
Compare
Fixed. |
Code looks nice and clean. Great work @SamVerschueren 👍 |
I started doing the refactoring work. I already did most of the schedule page. We might be able to extract some more components but I think this already is a nice improvement. I wasn't able to leverage
ChangeDetectionStrategy.OnPush
because the data isn't immutable. Might be a nice addition in the future.Please provide feedback of any kind. Namings (variables, functions, components, ...), code, ...
P.S. I don't like how Ionic handles styling. You don't have to specify the stylesheet in your component. It looks like it just takes all the SCSS files and compiles them into one big main file without view encapsulation. Might be possible though, have to look into it.