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

Unresponsive change button on foreign key in django 1.8 #408

Closed
tomazursic opened this issue Apr 25, 2015 · 16 comments
Closed

Unresponsive change button on foreign key in django 1.8 #408

tomazursic opened this issue Apr 25, 2015 · 16 comments
Labels
Milestone

Comments

@tomazursic
Copy link

Hi, using django-autocomplete-light 2.1.0rc3 with grappelli in Django 1.8, the buttons for changing or deleting related objects using a popup is unresponsive, it's missing the href attrubute.

Working default widget

    <div class="related-widget-wrapper">

        <select id="id_customer" name="customer"></select>

        <a id="change_id_stream_encoder" class="related-widget-wrapper-link change-related" title="Change selected Customer" data-href-template="/admin/address/customer/__fk__/?_to_field=id&_popup=1" href="/admin/address/customer/314/?_to_field=id&_popup=1">
            <img width="10" height="10" alt="Change" src="/static/admin/img/icon_changelink.gif"></img>
        </a>
        <a id="add_id_stream_encoder" class="related-widget-wrapper-link add-related" title="Add another Customer" href="/admin/address/customer/add/?_to_field=id&_popup=1"></a>

    </div>

Widget in django-autocomplete-light

    <div class="related-widget-wrapper">

        <span id="id_customer-wrapper" class=" autocomplete-light-widget customer single" data-widget-maximum-values="1" data-widget-bootstrap="normal" data-widget-ready="1"></span>

        <a id="change_id_customer" class="related-widget-wrapper-link change-related" title="Change selected customer" data-href-template="/admin/address/customer/__fk__/?_to_field=id&_popup=1">
            <img width="10" height="10" alt="Change" src="/static/admin/img/icon_changelink.gif"></img>
        </a>

        <a id="add_id_customer" class="related-widget-wrapper-link add-related" title="Add another customer" href="/admin/address/customer/add/?_to_field=id&_popup=1"></a>
        <div style="clear: both"></div>

    </div>

How to solve this issue?

@dalberto
Copy link

dalberto commented May 3, 2015

I think this is related to the way the "change" links are generated in Django. Django templates generate the data-href-template attribute, while the href attribute is populated client-side using the value of data-href-template (see here).

The incompatibility between django 1.8 and autocomplete-light exists because django's related-field JS expects the "change" and "delete" buttons/links for an object to be direct siblings of the select element (see here). Autocomplete-light generates a field wrapper that contains the select input element. When using autocomplete-light, the field wrapper itself is the sibling of the 'change' button/link, so the related-widget JS finds that there are no siblings for the select element and cannot populate the href attribute.

My temporary workaround is to include the following JS for the admin, which is modeled after related-widget-wrapper.js. It's quick and dirty but has worked as expected.

function updateAutoCompleteLinks() {
    var $this = $(this);

    var select = $this.children('.value-select').first();
    if (!select) {
        return;
    }

    var vals = select.val();
    var value;

    if (!vals || !vals.length) {
        value = '';
    } else {
        value = vals[0];
    }

    var links = $this.nextAll('.change-related, .delete-related');

    if (value) {
        links.each(function () {
            var elm = $(this);
            elm.attr('href', elm.attr('data-href-template').replace('__fk__', value));
        });
    } else links.removeAttr('href');
}

$(document).ready(function() {
    var container = $(document);
    container.on('change', '.autocomplete-light-widget', updateAutoCompleteLinks);
    container.find('.autocomplete-light-widget').each(updateAutoCompleteLinks);
});

@tomazursic
Copy link
Author

It works like a charm !
Thanks !

@blueyed
Copy link
Member

blueyed commented May 4, 2015

@dalberto
Thanks for the investigation and workaround!.

(for reference: it has been added in Django in django/django@0798874 (#13165 -- Added edit and delete links to admin foreign key widgets.))

I think this should get fixed in Django, e.g. by using the following "siblings" selector instead (in https://github.com/django/django/blob/stable/1.8.x/django/contrib/admin/static/admin/js/related-widget-wrapper.js#L4):

var siblings = $this.parent('.related-widget-wrapper').find('.change-related, .delete-related');

What do you think?

@tolmun
Can you check if that (or something similar) works?

@jpic
We probably want to have a test for this, e.g. by checking if the change links work in Django 1.8?
What workaround could we provide for this?

Would it be possible to not wrap (and hide) the select, but keep it in the same hierarchy, and only hide it?

@blueyed blueyed added the bug label May 4, 2015
@dalberto
Copy link

dalberto commented May 5, 2015

@blueyed
No problem! I think the fix you proposed would work and it's simple. However, I think changes would also be necessary in the selector that is used to listen to change events (here).

@blueyed
Copy link
Member

blueyed commented May 11, 2015

@dalberto
Do you feel like reporting this for Django?
What's your opinion/suggestion on working around this in autocomplete-light?

@dalberto
Copy link

@blueyed
I think it would be easier to add the fix in autocomplete-light. Perhaps as one of the static includes in the admin? While it would be ideal to have the fix in Django, I'm not sure it's generally an issue that will be worth addressing there.

@jpic
Copy link
Member

jpic commented May 14, 2015 via email

@jpic
Copy link
Member

jpic commented May 14, 2015 via email

@dalberto
Copy link

@jpic
Thanks!

I have not used M2M relationships in conjunction with autocomplete-light but vanilla Django M2M widgets don't have an "edit" button AFAIK, so I'm not sure if it's necessary. My experience and knowledge is limited for this use-case, so it is likely that I am wrong.

@jpic
Copy link
Member

jpic commented May 14, 2015

So far I'm trying to add the field and widget objects to
AutocompleteInterface.__init__(). It adds a bit of coupling, but should
make it easy to enhance user experience ie. #413 #239 / #361 #410.

@blueyed what do you think about coupling there ? It wouldn't be perfect,
but pretty useful.

http://yourlabs.org http://blog.yourlabs.org
Customer is king - Le client est roi - El cliente es rey.

@blueyed
Copy link
Member

blueyed commented May 14, 2015

@jpic
I have no opinion / objections in this regard. Go for it! :)

@jpic jpic modified the milestone: 2.2.0 May 15, 2015
@jpic
Copy link
Member

jpic commented May 17, 2015

Fixed in #415

We can't really couple django admin and autocomplete-light python code, because we'd need a way to get the admin site instance in the GET request, and there's really no simple way to my knowledge cause once the widget is displayed it's gone.

@jpic
Copy link
Member

jpic commented May 17, 2015

Closing in favor of #415.

@jpic jpic closed this as completed May 17, 2015
jpic added a commit that referenced this issue May 23, 2015
@jpic
Copy link
Member

jpic commented May 26, 2015

Could you try with 2.2.0rc1 which was released yesterday ?

Thanks.

@dalberto
Copy link

After testing, this fix seems to work when you choose a new autocomplete candidate for a field that is not populated. However, this fix does not work for a field that has already been populated, or when a new object is created via the "+" button in the admin.

@blueyed blueyed reopened this May 31, 2015
@jpic
Copy link
Member

jpic commented May 31, 2015

It really looks like a bad idea support their implementation of this feature, which relies on urls generated on the client side while it's common knowledge that's the not the right way to do with django.

I'd rather find a way to disable this feature, and enable AutocompleteModelTemplate by default instead of AutocompleteModelBase which has been doing this the right way literally for years.

This feature is also what broke the nice and modern positioning we had, and to support it we've had to go back to pre-1.10 positioning, because it nests overflow: hidden float clearing hack instead of using the standard clear: both which doesn't requires tons of hacks and works on IE.

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

4 participants