@mattab opened this Issue on June 14th 2010 Member

The feedback popup content is loaded by default.
Instead, the Feedback plugin controller should be called only when the 'Give us feedback!' link is clicked.

Fixing this will save 1 http request (see request).

@robocoder commented on June 25th 2010 Contributor

I'm not sure I can fix this.

As I recall, when I updated to using jQuery UI, the iframe approach was broken cross-browser; hence the current non-iframe approach.

Perhaps SteveG or JulienM could take a stab at it (with a fresh pair of eyes).

@sgiehl commented on June 25th 2010 Member

Isn't it enough to change the feedback.js like this?

$(function() {
    var feedback = $('a#topbar-feedback');
    if (feedback.size()) {
        var fbDiv = $('<div id="feedback-dialog"></div>').appendTo('body');

        $('#topbar-feedback').click(function() {
            if(fbDiv.html() == '') {
                $.get(feedback.attr('href'), function(data) {
                    fbDiv.html(data);
                });

                fbDiv.dialog({
                    title: feedback.html(),
                    bgiframe: true,
                    modal: true,
                    height: 480,
                    width: 500,
                    resizable: false,
                    autoOpen: false
                });
            }
            $('#feedback-faq').show();
            $('#feedback-form').hide();
            $('#feedback-sent').hide().empty();
            fbDiv.dialog('open');
            return false;
        });
    }

});
@mattab commented on June 25th 2010 Member

Close, but missing the image showing that the content is loading. Before it used to ajax load on click (rather than during init) like you suggest, but we need some image that shows that stuff is loading.

Alternatively we can display the standard 'Loading data...' (call to smarty {ajaxLoadingDiv id=feedbackLoading} eg.)

@sgiehl commented on June 25th 2010 Member

Well, I see.
What about this solution:

$(function() {
    var feedback = $('a#topbar-feedback');
    if (feedback.size()) {
        var fbDiv = $('<div id="feedback-dialog"></div>').appendTo('body');

        $('#topbar-feedback').click(function() {
            if(fbDiv.html() == '') {
                fbDiv.html('<div id="feedback-loading"><img alt="" src="themes/default/images/loading-blue.gif"> '+translations.CoreHome_Loading_js+'</div>');
            }
            if($('#feedback-loading' ,fbDiv).length) {
                $.get(feedback.attr('href'), function(data) {
                    fbDiv.html(data);
                });

                fbDiv.dialog({
                    title: feedback.html(),
                    bgiframe: true,
                    modal: true,
                    height: 480,
                    width: 500,
                    resizable: false,
                    autoOpen: false
                });
            }
            $('#feedback-faq').show();
            $('#feedback-form').hide();
            $('#feedback-sent').hide().empty();
            fbDiv.dialog('open');
            return false;
        });
    }

});
@sgiehl commented on June 28th 2010 Member

Any further suggestions or shall I commit that change to trunk?

@mattab commented on June 28th 2010 Member

assuming you've tested it, looks good to me

@robocoder commented on June 29th 2010 Contributor

(What was I thinking?)

Good work. Check it in.

@sgiehl commented on June 29th 2010 Member

(In [2398]) fixes #1425 load feedback popup only if required

This Issue was closed on June 29th 2010
Powered by GitHub Issue Mirror