Skip to content

Conversation

@dszczyt
Copy link
Contributor

@dszczyt dszczyt commented Jun 18, 2015

Your directives can be much lighter. Here is an example. I've added some tests to be sure it does the job. I can help to rewrite your directives like this one. Does anybody have suggestions to improve it ?

Copy link
Member

Choose a reason for hiding this comment

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

Could you replace double quotes by single ones? I really dislike using backlash character 😇

@jspdown
Copy link
Member

jspdown commented Jul 7, 2015

Your code refactoring on this component is definitely better than the previous implementation. I want it merged!

As mentioned in my previous comment, we prefer single quote for Javascript and double quote for Html.

@dszczyt
Copy link
Contributor Author

dszczyt commented Jul 8, 2015

OK it's done. Glad you like it !

@dszczyt
Copy link
Contributor Author

dszczyt commented Jul 8, 2015

FYI, I wanted to keep your original syntax. But I think the "type" and "size" args are not necessary anymore : you could use <checkbox class="slider large" ng-model="myVar">[label]</checkbox> to generate a large slider... What do you think about it ?

@jspdown
Copy link
Member

jspdown commented Jul 15, 2015

@dszczyt, type and size are useless if we use semantic-ui classes. In my mind, angular-semantic-ui should only provide a way to easily use semantic-ui dynamic components on an AngularJs web application.
So, we should depend on their css classes every time we can. I definitely agree with you.

@dszczyt
Copy link
Contributor Author

dszczyt commented Jul 27, 2015

This is how it looks like. This directive becomes lighter and lighter...

jspdown added a commit that referenced this pull request Jul 28, 2015
Checkbox code refactoring
@jspdown jspdown merged commit c1c64ff into angularify:master Jul 28, 2015
@jspdown
Copy link
Member

jspdown commented Jul 28, 2015

Great! Thanks @dszczyt
I will try to release a new version on Bower this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants