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

Page demos problems in Firefox 44 #16836

Closed
pocesar opened this issue Jan 31, 2016 · 19 comments
Closed

Page demos problems in Firefox 44 #16836

pocesar opened this issue Jan 31, 2016 · 19 comments
Labels

Comments

@pocesar
Copy link

pocesar commented Jan 31, 2016

When using the first input in the page https://leaverou.github.io/awesomplete/ it shows as this (double tooltip):

fireshot capture 2 - awesomplete_ ultra lightweight high_ - https___leaverou github io_awesomplete_

then after selecting the first item:

fireshot capture 3 - awesomplete_ ultra lightweight high_ - https___leaverou github io_awesomplete_

and down with the combobox dropdown, the toggle is misaligned

fireshot capture 1 - awesomplete_ ultra lightweight high_ - https___leaverou github io_awesomplete_

@vlazar vlazar added the bug label Jan 31, 2016
@vlazar
Copy link
Collaborator

vlazar commented Jan 31, 2016

Awesomplete was included twice by mistake. This created 2 completer instances. Fixed in 6c20ebe

@vlazar
Copy link
Collaborator

vlazar commented Jan 31, 2016

Closing this one since the first bug is fixed and second one is in separate issue now #16838

@vlazar vlazar closed this as completed Jan 31, 2016
@pocesar
Copy link
Author

pocesar commented Jan 31, 2016

👍

@TxHawks
Copy link
Contributor

TxHawks commented Jan 31, 2016

This brings up an issue though.

Should Awesomplete check if an input is already initialized before
initializing?

Could be done by checking the Awesomplete.all array

On Sun, Jan 31, 2016, 12:34 Vladislav Zarakovsky [email protected]
wrote:

Closing this one since the first bug is fixed and second one is in
separate issue now #16838
#16838


Reply to this email directly or view it on GitHub
#16836 (comment)
.

@vlazar
Copy link
Collaborator

vlazar commented Jan 31, 2016

Yes.

@LeaVerou
Copy link
Owner

LeaVerou commented Feb 1, 2016

I’m not so sure. It introduces a performance overhead on every use for little benefit (since including it twice is essentially the author's bug).
I'm not necessarily saying no, I'm saying "yes" is not that obvious to me.

@vlazar
Copy link
Collaborator

vlazar commented Feb 1, 2016

Not necessarily performance overhead. Including script twice is an obvious error. Creating new instance for input, which already has awesomplete maybe not.

Anyways, won't implement if it adds significant performance overhead or complicates code to much.

@TxHawks
Copy link
Contributor

TxHawks commented Feb 1, 2016

I think the easiest way to do this is to check is input is the same as
one of the inputs in _.all just before element creation.

I don't think the performance overhead should be significant, as everything
is already cached, so there is no unneeded DOM querying.

I think the check can probably be done pretty neatly with _.all.reduce()

On Mon, Feb 1, 2016, 18:50 Vladislav Zarakovsky [email protected]
wrote:

Not necessarily performance overhead. Including script twice is an obvious
error. Creating new instance for input, which already has awesomplete maybe
not.

Anyways, won't implement if it adds significant performance overhead or
complicates code to much.


Reply to this email directly or view it on GitHub
#16836 (comment)
.

@LeaVerou
Copy link
Owner

LeaVerou commented Feb 5, 2016

Why reduce and not indexOf?

@ghost
Copy link

ghost commented Feb 5, 2016

Fuck you! Stop emailing me

Thank you,
Stacey

On Feb 4, 2016, at 7:23 PM, Lea Verou [email protected] wrote:

Why reduce and not indexOf?


Reply to this email directly or view it on GitHub.

@LeaVerou
Copy link
Owner

LeaVerou commented Feb 5, 2016

LOL, Stacey, nobody is emailing you. You (or someone using your email) has subscribed to this conversation. Just press unsubscribe.

@ghost
Copy link

ghost commented Feb 5, 2016

I would have 3,823 emails ago. There is no unsubscribe. And thank you for not taking my message personally. Apple should allow blocking of email rather than just junk

Thank you,
Stacey

On Feb 4, 2016, at 7:25 PM, Lea Verou [email protected] wrote:

LOL, Stacey, nobody is emailing you. You (or someone using your email) has subscribed to this conversation. Just press unsubscribe.


Reply to this email directly or view it on GitHub.

@LeaVerou
Copy link
Owner

LeaVerou commented Feb 5, 2016

The notifications you are getting are automated for every message posted on this conversation. It's not an email conversation, the emails are just notifications that you can reply to. The actual conversation is on the website. Note the "View it on Github" link under every message? That takes you to the website.
However, to unsubscribe, I imagine you need to log in, so if you didn't register, you need to figure out who did using your email.

@ClausMandal
Copy link

Lea you radiate professionalism left and right! Well done.

@TxHawks
Copy link
Contributor

TxHawks commented Feb 5, 2016

You could either use reduce or all.map().indexOf().

You can't use indexOf() directly, because Awesomplete.all is an array of objects, each with a property of input, which is the HTML element

@LeaVerou
Copy link
Owner

LeaVerou commented Feb 5, 2016

Yes, that's what I meant. I think it would be more readable than reduce.

@TxHawks
Copy link
Contributor

TxHawks commented Feb 5, 2016

Sure thing. Want me to submit a PR?

@LeaVerou
Copy link
Owner

LeaVerou commented Feb 5, 2016

Eh, why not. :)

@vlazar
Copy link
Collaborator

vlazar commented Feb 5, 2016

The short answer would be - tests. I need to figure out a clean way to test Awesomplete's addition via script tag to the page. This is pretty much the only part not covered by tests yet.

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

No branches or pull requests

5 participants