search

friendica / red

The Red Matrix
MIT License
214 stars 50 forks source link

more regressions: autocomplete #869

Closed git-marijus closed 9 years ago

git-marijus commented 9 years ago

3 things:

  1. autocomplete in /mail/new seems to have a similar issue as nav-search-autocomplete used to have: when typing a name only the first or sometimes two characters are submitted to the query. starting over a second query fails completely. only after a page reload autocomplete works again. again only one chance.
  2. there seems to be an issue having two autocompletes in one page: after using ac in /mail/new once the nav-search-autocomplete does not work anymore... it works vice versa though... but again after using /mail/new autocomplete nav-search-autocomplete stops working.
  3. nav-search-autocomplete position should be fixed (since the panel is fixed). also it goes off the screen on the right side. we used to have the possibility to attach an id to every autocomplete dropdown to have a chance to style it. this does not seem to be the case anymore.
pafcu commented 9 years ago

I can't reproduce issues "0".

Issue 1 is due to a javascript error if a group is displayed in the autocomplete, so not related to two autocompletes. Fixed.

I'm not sure what is meant by issue two. Fixed how? Do you mean that the menu should not be displayed at the end of the cursor, but instead always at the same position, at the start of the search box?

"3". Do you mean attach an id to the menu itself?

pafcu commented 9 years ago

BTW, when running into problems with javascript libraries, it's usually a good idea to check the javascript error console, as it often contains useful information.

git-marijus commented 9 years ago

fixed in the sense of css position: fixed; since it belongs to the panel it should not scroll with the page. width should not go offscreen... according to this ( https://github.com/yuku-t/jquery-textcomplete/blob/master/doc/how_to_use.md ) doc it should be possible to attach a class to the dropdown... so should be possible to style each dropdown separately...

pafcu commented 9 years ago

Sure. That's already there in the code, currently it uses the acpopup class for all the dropdowns, but this could be changed easily.

pafcu commented 9 years ago

If it's supposed to stay with the navbar, why not make it relative to the navbar? That's the correct solution, IMHO, because it continues to work even if the navbar bahviour is changed.

pafcu commented 9 years ago

Bug filed upstream: https://github.com/yuku-t/jquery-textcomplete/issues/151

git-marijus commented 9 years ago

yeah it would be good to have a unique class or id for each ac-dropdown to have the ability to style each seperately...

pafcu commented 9 years ago

While possibly useful, is there a specific use case for this right now? Personally I think it would be better to keep all the dropdowns similar, and fix bugs like not being fixed the proper way, in the library itself (setting a fixed position in CSS is just a workaround).

git-marijus commented 9 years ago

i dont think that styling and positioning with css is a workaround... do you?

pafcu commented 9 years ago

In the above case, yes. And I already told you why. It's a bug in the library, and should be fixed in the library.

Now, can you mention some use case for being able to style and position of a specific dropdown, that is not a workaround?

If it's a dropdown that's somehow different from other dropdowns, you shouldn't use a generic dropdown for it, use a more specialized version, and in that dropdown, use another CSS class. That way, if someone uses that dropdown somewhere else too it also gets styled automatically. Cool, huh?

I honestly can't think of a legitimate use of being able to specify an id for the dropdown right now. Especially not for styling purposes. I can see a drawback however: people putting in workarounds, which will just cause bugs to appear later on.

Consider if you now put the dropdown position to fixed using CSS. Now, if someone decides to make the navbar non-fixed, but doesn't know that they also need to change dropdown position (and why would they?) you'd just end up with the reverse of the bug described above. If, on the other hand, the library is modified to handle the case of fixed elements, things "just work", no matter how you style the navbar.

Setting the dropdown position using CSS is probably easier in the short run. But in the long run, the best option is to fix the library upstream so that not every other project also has to come up with a workaround.

git-marijus commented 9 years ago

feel free to fix it properly when upstream has fixed it...

pafcu commented 9 years ago

Fixed, and hopefully included in upstream soon. https://github.com/pafcu/jquery-textcomplete/commit/059b599536592f42abe34426cd92c2ebf4b0a1c8

ghost commented 9 years ago

This issue was moved to redmatrix/redmatrix#299