Meteor-Community-Packages / meteor-autocomplete

Client/server autocompletion designed for Meteor's collections and reactivity.
https://atmospherejs.com/mizzao/autocomplete
MIT License
350 stars 109 forks source link

position: "top" doesn't work when using tokenless completion #75

Closed jfly closed 9 years ago

jfly commented 9 years ago

It's pretty clear that this was intentional, as there's a comment right here: https://github.com/mizzao/meteor-autocomplete/blob/master/autocomplete-client.coffee#L306 saying "# TODO allow this to render top as well, and possibly used in textareas?"

Are there plans to add support for this this? I was able to monkeypatch a fix locally, but could create a pull request if you like.

mizzao commented 9 years ago

Sure, why don't you share what you did for the monkey patch and I'll try and work it in.

jfly commented 9 years ago

Here's my monkeypatch (source):

// Hacky workaround for https://github.com/mizzao/meteor-autocomplete/issues/75
function isWholeField() {
  return true;
}
AutoComplete.prototype.positionContainer = function() {
  var offset, pos, position, rule;
  position = this.$element.position();
  rule = this.matchedRule();
  if((rule !== null) && isWholeField(rule)) {
    pos = {
      left: position.left,
      //JFLY top: position.top + this.$element.outerHeight(),
      width: this.$element.outerWidth()
    };
    //JFLY
    offset = { top: 0 };
    if(this.position === "top") {
      pos.bottom = this.$element.offsetParent().height() - position.top - offset.top;
    } else {
      pos.top = position.top + offset.top + parseInt(this.$element.css('font-size'));
    }
    //JFLY
  } else {
    offset = getCaretCoordinates(this.element, this.element.selectionStart);
    pos = {
      left: position.left + offset.left,
    };
    if(this.position === "top") {
      pos.bottom = this.$element.offsetParent().height() - position.top - offset.top;
    } else {
      pos.top = position.top + offset.top + parseInt(this.$element.css('font-size'));
    }
  }
  return this.tmplInst.$(".-autocomplete-container").css(pos);
};
mizzao commented 9 years ago

I see, you just copied the stuff from the bottom. Does that actually work properly?

i.e. using outerHeight is pretty important for the menu to show up right below in the whole field case.

jfly commented 9 years ago

It seems to =)

mizzao commented 9 years ago

Great, released in 0.5.1.

Also updated the demo app to allow this property to be switched.

jfly commented 9 years ago

Just tried out 0.5.1, it works, but the position of the dropdown (ok, dropup =)) is a bit weird. Note how in my hack, I was hardcoding offset = { top: 0 } (source):

2015-02-27_943x970_laptor

In 0.5.1, offset.top is nonzero. With a nonzero offset.top, I'm seeing the dropup overlap a bit with my input field:

2015-02-27_945x1003_laptor

edit: Added screenshots (note the different css bottom attributes).

jfly commented 9 years ago

Any update here? I'm not wild about how things look as of 0.5.1

jfly commented 9 years ago

Bump =)

mizzao commented 9 years ago

Sorry, things are a little busy here. I went that way because it was just cleaner code. Otherwise it's asymmetric - the dropup doesn't overlap with the top of the box, but the dropdown does for a textarea. I can think about how to make this better later, but maybe you should just go ahead and put your preference into your own fork for now, and just use that.

jfly commented 9 years ago

Ah, I see. I didn't realize this was intentional. I do think things look better if the textbox doesn't get overlapped, but I don't feel strongly enough about this to maintain a fork. Thanks!