Skip to content
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

Return nothing if popup is not the desired purpose #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Return nothing if popup is not the desired purpose #42

wants to merge 1 commit into from

Conversation

RomainLanz
Copy link
Contributor

Hi,

There are a lot of errors on this directive, like the use of eval() and many other little things...

var title = popup_meta_data['title'];
if (title == undefined)
    title = ''

can be...

var title = popup_meta_data['title'] || '';

But, the main problem here is that popup is to generic! I've other package who use this HTML option (data-popup) and without a test to be sure that popup_meta_data is not undefined I've got a lot of error in my console.

This is just a hack. But I think you need to remove completly this component and use some convention like asm-popup or whatever (sm-* is used by the real Semantic-UI Angular component).

@jspdown
Copy link
Member

jspdown commented Jul 7, 2015

I never looked at this component before. I agree with you, this directive is full of errors and pretty ugly 😄
We should use more the semantic-ui javascript library. I really dislike all calculus we make in this component.
We could replace it by:

$('#element').popup();

and use all parameters the library provide: http://semantic-ui.com/modules/popup.html#/usage

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

Successfully merging this pull request may close these issues.

2 participants