-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Conversation
Few things. No backquotes on title. The demo html needs to be updated to use the prefix and the readme if mentions (and it does in this case) need to be updated with prefixes as well. The main demo app has tabs so that needs to be fixed (I can do that in a separate commit, no problem). My biggest problem here is the In Accordion we have our own directive for that and for this one, we have sort of a hack. My point in here is that in here we don't have So I think that we need to prefix that as well but also supporting it without prefix for the deprecated one. Thoughts on this @wesleycho ? |
@Foxandxss, ok. the title can be fixed on the merge. demo has been updated. |
}; | ||
}]) | ||
|
||
.directive('tabHeadingTransclude', ['$log', '$tabSuppressWarning', function($log, $tabSuppressWarning) { |
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.
Fix indentation
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.
Yup, forgot to mention that too.
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.
done. there were a couple of lines that needed indent fixes.
I think my comment got lost. Fixed indentation in a couple of spots. |
This LGTM - @Foxandxss ? |
Hang on, there's some duped lines in the second test. I'll remove and repush. |
The template was getting compiled and digested twice which was resulting in double the number of |
Alrighty merged. Changed $tabSuppress to $tabsSuppress and there was a missing prefix on the demo. |
No description provided.