feat(menu): Add BEM classes #297
Conversation
You know the drill. - Updated BEM classes, including root, header, footer - Updated docs BREAKING CHANGE: The default template now has the count element inside the link, not outside.
| @@ -47,7 +47,7 @@ function menu({ | |||
| var containerNode = utils.getContainerNode(container); | |||
| var usage = 'Usage: menu({container, facetName, [sortBy, limit, cssClasses.{root,list,item}, templates.{header,item,footer}, transformData, hideWhenResults]})'; | |||
|
|
|||
| if (container === null || facetName === null) { | |||
| if (!container || !facetName) { | |||
There was a problem hiding this comment.
Why? :) Was there an edge case that we didn't support in this case?
There was a problem hiding this comment.
Nope, it's a line that had been forgotten in a previous refactoring I guess.
We used to pass default parameters as container = null, facetName = null and we then decided to simply pass container, facetName with no value and test if they were falsy.
Here we had done only half the job so I changed the second part :)
There was a problem hiding this comment.
Oh we decided that? Cool then. :)
There was a problem hiding this comment.
It was more an oral decision that anything written down actually ;)
There was a problem hiding this comment.
AH :) Be careful if we are to have a global team anytime :)
There was a problem hiding this comment.
Yeah I know, we'll have to have a place to write those styleguide/standards somewhere soon. Just a wiki page would do.
|
LGTM! 👍 |
You know the drill.
BREAKING CHANGE: The default template now has the count element inside
the link, not outside.