Conversation
| }, | ||
| controls: null, | ||
| tooltipTO: null, | ||
| buildModal:function(title,content,altFunc) { |
There was a problem hiding this comment.
You don't handle cases when, for some reasons, content is wrong type. Could you add a type check handle for the function to proceed?
There was a problem hiding this comment.
The issue is, that I'd need to typecheck both content itself as well as any children it might have.
content can be used as a string or the usual configuration-object, throwing anything else out, that doesn't see to fit is imo the best in that case.
I fear checking the whole content-tree might just be a bit too much overhead.
There was a problem hiding this comment.
A simple example: buildModal(null, null, null) -> buildModal(undefined, undefined, undefined). Because API expose these as a public method in both scenarios popup should draw close sign, and have empty body and empty title.
|
We really need to deal with spaces somehow. I use Idea Jetbrains and our spacing policy is totally different :) I really don't think that one space before and after a function name/call is a good idea to use in JS, it's my humble opinion. Would be good idea to reduce usage of parseInt function. |
|
Regarding spaces: I consider sourcecode with some spaces more readable, but I agree that the finally delivered code needs to be as small as possible. |
|
added an else-case if they didn't match the required format and an output that should be sufficiently obvious so the caller understands what is missing. |
|
Sorry, missed that part. Looks very nice ;) Merging. |
|
Could you update your branch with master? |
|
sorry, forgot to push :D |
|
legit 👍 |
2 participants
Fixes #17 provide framework and basic styling
Fixes #1 fixing possible bug