search

Semantic-Org / Semantic-UI-Angular

Semantic UI Angular Integrations
MIT License
557 stars 117 forks source link

sm-divider directive #6

Closed m0t0r closed 9 years ago

m0t0r commented 9 years ago

@caitp please take a look at this pretty simple directive

caitp commented 9 years ago

looks okay to me --- might want to also add a case for the grid divider option

m0t0r commented 9 years ago

Well, as I see for the grid case they actually have it on the grid itself as ui divided grid. I am not sure how it could be also considered on this directive?

caitp commented 9 years ago

Beats me ;) I think it's fine, although it seems like it might not be worth pushing through the compile process for such a minor component

m0t0r commented 9 years ago

The reason is when divider has just ui divider classes it does not format internal content such as word "or" or something, it does when it has actual classes either vertical or hortizontal so that we can write 

<sm-divider vertical>Or</sm-divider>

instead of 

<sm-divider class="vertical">Or</sm-divider>

What do you think is it worth to compile or native class style is better ?

caitp commented 9 years ago

Making it a directive means you're compiling regardless

m0t0r commented 9 years ago

Oh, I did not understand what exactly you ment. Well, I took a look at material design and they actually have it as a directive. let's keep it as a directive for a while?