Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Regression: feedback popup content is loaded by default #1425

Closed
mattab opened this Issue · 8 comments

3 participants

Matthieu Aubry Anthon Pang Stefan Giehl
Matthieu Aubry
Owner

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).

Anthon Pang
Collaborator

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).

Stefan Giehl
Collaborator

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;
        });
    }

});

Matthieu Aubry
Owner

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.)

Stefan Giehl
Collaborator

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;
        });
    }

});
Stefan Giehl
Collaborator

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

Matthieu Aubry
Owner

assuming you've tested it, looks good to me

Anthon Pang
Collaborator

(What was I thinking?)

Good work. Check it in.

Stefan Giehl
Collaborator

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

Matthieu Aubry mattab added this to the Piwik 0.6.4 milestone
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.