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

clonePosition() bug in prototype.js ver 1.7.3 #2643

Closed
kiatng opened this issue Oct 3, 2022 · 4 comments
Closed

clonePosition() bug in prototype.js ver 1.7.3 #2643

kiatng opened this issue Oct 3, 2022 · 4 comments

Comments

@kiatng
Copy link
Contributor

kiatng commented Oct 3, 2022

Summary (*)

Ref issue #1497. I experienced the bug reported here. I use Ajax.Autocompleter() in

Autocompleter.Base = Class.create({

and expected the div element to show up below the input element, but the div element top position was incorrectly calculated in clonePosition()

function clonePosition(element, source, options) {

Examples (*)

See example here to test. Copy paste in case the link no longer work:

HTML

<input type="text" id="autocomplete" name="autocomplete_parameter"/>
<div id="autocomplete_choices" class="autocomplete"></div>

Javascript

new Ajax.Autocompleter("autocomplete", "autocomplete_choices", "/url/on/server", {});

CSS
The styling of the div and the returned UL are important.
Applying a visual cue that an item is selected allows the user to take advantage of the keyboard navigation of the dropdown and adding background colors, borders, positioning, etc to the div (as the demo does) allows the UI element to stand out. The CSS from the demo applied to the examples would be:

div.autocomplete {
  position:absolute;
  width:250px;
  background-color:white;
  border:1px solid #888;
  margin:0;
  padding:0;
}
div.autocomplete ul {
  list-style-type:none;
  margin:0;
  padding:0;
}
div.autocomplete ul li.selected { background-color: #ffb;}
div.autocomplete ul li {
  list-style-type:none;
  display:block;
  margin:0;
  padding:2px;
  height:32px;
  cursor:pointer;
}

Proposed solution

Replace clonePosition() with this:

  function clonePosition(element, source, options) {
    options = Object.extend({
      setLeft:    true,
      setTop:     true,
      setWidth:   true,
      setHeight:  true,
      offsetTop:  0,
      offsetLeft: 0
    }, options || {});

    source  = $(source);
    element = $(element);
    var p, delta, layout, styles = {};

    if (options.setLeft || options.setTop) {
      p = Element.viewportOffset(source);
      delta = [0, 0];
      if (Element.getStyle(element, 'position') === 'absolute') {
        var parent = Element.getOffsetParent(element);
        if (parent !== document.body) delta = Element.viewportOffset(parent);
      }
    }

    var pageXY = { x: document.body.scrollLeft, y: document.body.scrollTop };

    if (options.setWidth || options.setHeight) {
      layout = Element.getLayout(source);
    }

    if (options.setLeft)
      styles.left = (p[0] + pageXY.x - delta[0] + options.offsetLeft) + 'px';
    if (options.setTop)
      styles.top  = (p[1] + pageXY.y - delta[1] + options.offsetTop)  + 'px';

    if (options.setWidth) {
      styles.width = layout.get('width')  + 'px';
    }
    if (options.setHeight) {
      styles.height = layout.get('height') + 'px';
    }

    return Element.setStyle(element, styles);
  }
@ADDISON74
Copy link
Contributor

I don't think there will be a 1.8.0 version, so you can propose a PR to integrate it into the existing code. Maybe we should take a look at the reported issues and proposed PRs for this library to bring our version to a better shape. Not being an active project, we will have to patch it here, it is not like ZF1-Future.

@ADDISON74
Copy link
Contributor

ADDISON74 commented Oct 3, 2022

This one seems to be a serious bug: prototypejs/prototype#355.

There is a PR in their repository but it was not merged yet. We should integrated it in advanced.

@ADDISON74
Copy link
Contributor

I am closing it because it was solved by PR #2669, but I will analyze this library in more detail and its issues already reported on the official page. I have in plan to take over some PRs to bring them here.

@Neustradamus
Copy link

To follow!

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

No branches or pull requests

3 participants