-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HTML content opener #9
Conversation
Hi, |
@@ -129,8 +131,10 @@ | |||
|
|||
if ( setting.opener.active ) | |||
{ | |||
if ( ! setting.opener.open ) throw 'Url for opener.open image is not defined'; | |||
if ( ! setting.opener.close ) throw 'Url for opener.close image is not defined'; | |||
if ( ! setting.opener.as || (setting.opener.as !== 'img' && setting.opener.as !== 'html') ) |
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.
This should be enough
if ( setting.opener.as !== 'img' && setting.opener.as !== 'html' )
sholdn't be?
Done with your 2 feedbacks, let me know if there is more I can do |
Hi I have it still on mind. But I am not 100% sure and have not enough time. |
Ok noted. |
@@ -43,6 +43,7 @@ | |||
}, | |||
opener: { | |||
active: false, | |||
as: 'img', |
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.
We can leave it without as. Only if user want to switch to html he set it to "html". May be that param could be named as html: true,
Later in code(line 134) I would remove that condition. Next two conditions should be enough.
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.
Now I see that if we leave it without as and you will check it you must do
If ( o.as && o.as === 'html' )
So there are two conditions but I would like to have one
Solution is to have as as default and check
if ( o.as ==== 'html' )
or param will be optional like asHtm: true and so condition will be
If ( o.asHtml )
I would like to have that last option.
Actually I used |
For the future is important to have one place where you easy find and change this implementation. It solves setOpenClose() function. Conditions if ( html ) and if ( as === 'html' ) have the same efect. If you want to add cssClass if ( html ) will be false. No problem. Its easy to extend it in the future. |
About that future. I think html will change current url and if you have html elements you can add class directly to them. |
!!!!!! You are right with that opener[state] !!!!!!!! My head is out of order... |
Thank you for good idea. I made it little different. |
Add functionality to use HTML code for the open and close openers content by setting
opener.as = 'html'
, allowing for example to use font-awesome icons by writing options like this :