feat(): add toolbar component #146
Conversation
googlebot
added
the
cla: yes
src/components/toolbar/toolbar.html
Outdated
| @@ -0,0 +1 @@ | |||
| <ng-content></ng-content> | |||
There was a problem hiding this comment.
When the template of a component is just <ng-content>, it can be a @Directive (no template) instead of a @Component
devversion
force-pushed
the
wip/toolbar-component
branch
8 times, most recently
from
912e639 to
7f77d64
Compare
src/components/toolbar/toolbar.scss
Outdated
| // Fills the remaining space of the toolbar row. | ||
| .md-fill { | ||
| flex: 1 1 auto; | ||
| } |
There was a problem hiding this comment.
I'm tempted to say that we eliminate this class and just people apply the styles they want to apply. I don't want to end up with a giant bag of classes that can be applied to different components in different contexts.
There was a problem hiding this comment.
Well, I'll remove that, but IMO there should be a class / element which fills the remaining space in the toolbar row. Otherwise the user has to apply the flex fill himself.
|
Left a couple of comments and also added a screenshot to your PR description. Looking good! I like the API. Aside from the comments, it just needs @robertmesserle can you also give this a look? |
devversion
force-pushed
the
wip/toolbar-component
branch
2 times, most recently
from
71d9719 to
c48184a
Compare
|
@jelbourn Completed your suggestions and added a |
jelbourn
added
pr: needs review
and removed
in progress
|
|
||
| ### Example | ||
| ```html | ||
| <md-toolbar [color]="myColor"> |
There was a problem hiding this comment.
Start off with a simple example of the toolbar with just a single row (most common case), and then say something like "The toolbar also supports multiple rows" with the second example.
src/components/toolbar/README.md
Outdated
| ``` | ||
|
|
||
| ### Alignment | ||
| The `md-toolbar` component doesn't provide any API to align the items probably in the row. |
There was a problem hiding this comment.
I'd omit this line, and just go straight into saying that the toolbar content can be directly styles with flex, and then show the example.
There was a problem hiding this comment.
Just wanted to clarify that there's no public API by the component itself.
devversion
force-pushed
the
wip/toolbar-component
branch
2 times, most recently
from
354a50c to
a940153
Compare
devversion
force-pushed
the
wip/toolbar-component
branch
from
cb1d594 to
c5a0515
Compare
|
LGTM |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
5 participants
--- WIP ---
The design doc isn't finished yet, but as discussed with @jelbourn, I'll submit that PR, so I can retrieve some feedback.
Open Points:
PS: Issue intentionally not referenced yet.