Opened 4 years ago

Closed 4 years ago

#1425 closed Bug (fixed)

Regression: feedback popup content is loaded by default

Reported by: matt Owned by:
Priority: major Milestone: Piwik 0.6.4
Component: UI - UX (AngularJS, twig, less) Keywords:
Cc: Sensitive: no

Description

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

Change History (8)

comment:1 Changed 4 years ago by vipsoft (robocoder)

  • Owner vipsoft deleted

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

comment:2 Changed 4 years ago by SteveG (sgiehl)

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

comment:3 Changed 4 years ago by matt (mattab)

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

comment:4 Changed 4 years ago by SteveG (sgiehl)

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

comment:5 Changed 4 years ago by SteveG (sgiehl)

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

comment:6 Changed 4 years ago by matt (mattab)

assuming you've tested it, looks good to me

comment:7 Changed 4 years ago by vipsoft (robocoder)

(What was I thinking?)

Good work. Check it in.

comment:8 Changed 4 years ago by SteveG (sgiehl)

  • Resolution set to fixed
  • Status changed from new to closed

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

Note: See TracTickets for help on using tickets.