Autocomplete fixes and enhancements #533
Conversation
… to with the bind() function and the initialization function doesn't need to be modified. Moved the autocomplete window to the form so that the bounding rect is not needed thus avoiding positioning problems e.g. when window is scrolled while suggestions are loading. Align suggestions when window is resized. Set the initial content with .html(). Use the greater of input width and form width for the maximum autocomplete list width instead of the arbitrary * 2 width.
|
Thanks for these -- I'll wait on merging until @crhallberg has a chance to take a look. Are you still having trouble with clicking entries on a Mac, or has that problem been resolved at this point? (We tried to reproduce the issue over here but could not). |
|
Looks like the clicking problem cannot be reproduced anymore, so I suppose another change affected that. I'll let you know if I can reproduce it again. |
|
Thanks! |
|
Good catches! I merged these in manually since there was an update to autocomplete.js between this fork and now. |
|
@crhallberg, the change that fixed positioning was not merged. Is there a reason for that, e.g. a problem I didn't account for? The current version in vufind-org/master is still not working properly if the position of the input field changes in the viewport while suggestions are loading. |
|
@EreMaijala The fixes you proposed were based on a version before I fixed the scrolling problem in general (I wasn't taking scrolling into consideration). When comparing the two methods, I intentionally kept mine for performance reasons (less calls to and searches in the DOM). I just double checked by scrolling all over while loading and typing and the issue is definitely resolved. Thank you again! |
|
@crhallberg Your version works with Chrome and Safari but not with Firefox (Mac) or Internet Explorer 9 (Windows 7). |
|
I fixed the IE 9 bugs but I cannot produce any Firefox bugs, even on a Mac. |
|
@crhallberg Now it works with Firefox and IE 9, but doesn't work with Chrome or Safari anymore. |
|
@crhallberg Unless there's a compelling reason to keep the autocomplete results under the body element, please consider again my proposed version. If you had it in the form you wouldn't have to take the scroll position into account at all, and I don't believe having a couple DOM calls here is really a limiting factor. |
|
My apologies. I'm not trying to be stubborn, I thought that putting the autocomplete results into a form would prevent it from moving around the page properly, since multiple inputs use the same results div. I merged in your changes completely and tested. It all works great. I only upped the min-width to the input width for style preferences. Sorry again for the long draw on this. I'm merging it in now. |
|
No problem, and thanks for working on it! :) |
Switched to trigger() for the selection event so that it can be bound to with the bind() function and the initialization function doesn't need to be modified.
Moved the autocomplete window to the form so that the bounding rect is not needed thus avoiding positioning problems e.g. when window is scrolled while suggestions are loading.
Align suggestions when window is resized.
Set the initial content with .html().
Use the greater of input width and form width for the maximum autocomplete list width instead of the arbitrary * 2 width.