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

Jump to bottom

Regression: feedback popup content is loaded by default #1425

Closed
mattab opened this issue Jun 14, 2010 · 8 comments
Closed

Regression: feedback popup content is loaded by default #1425

mattab opened this issue Jun 14, 2010 · 8 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone
0.6.4

Comments

@mattab
Copy link
Member

mattab commented Jun 14, 2010

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
Copy link
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
Copy link
Member

sgiehl commented Jun 25, 2010

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
Copy link
Member Author

mattab commented Jun 25, 2010

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
Copy link
Member

sgiehl commented Jun 25, 2010

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
Copy link
Member

sgiehl commented Jun 28, 2010

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

@mattab
Copy link
Member Author

mattab commented Jun 28, 2010

assuming you've tested it, looks good to me

@robocoder
Copy link
Contributor

(What was I thinking?)

Good work. Check it in.

@sgiehl
Copy link
Member

sgiehl commented Jun 29, 2010

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

@mattab mattab added this to the Piwik 0.6.4 milestone Jul 8, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug For errors / faults / flaws / inconsistencies etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Milestone
0.6.4
Development

No branches or pull requests
3 participants
@mattab @robocoder @sgiehl