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

Remove duplicated spinner #38

Closed
wants to merge 1 commit into from
Closed

Remove duplicated spinner #38

wants to merge 1 commit into from

Conversation

rodfersou
Copy link
Member

closes #35

@hvelarde
Copy link
Member

hvelarde commented Mar 1, 2017

I don't think this is the right way to fix it; the spinner is duplicated, so probably we don't need to redeclare it in this package.

@rodfersou
Copy link
Member Author

rodfersou commented Mar 1, 2017

@hvelarde it is true for Plone 4.3.x, but for Plone 5.x we will miss the spinner if I remove it

@rodfersou rodfersou changed the title Disable spinner for Plone < 5.x Use Plone default spinner Mar 2, 2017
@rodfersou
Copy link
Member Author

@rodfersou
Copy link
Member Author

plone/mockup#745

Copy link
Member

@hvelarde hvelarde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is simpler: our spinner declaration activates Plone's default spinner and then it's shown twice; I don't want to use the default spinner; I want to show only ours.

@rodfersou
Copy link
Member Author

Ok, let's start again :)

@rodfersou rodfersou changed the title Use Plone default spinner Remove duplicated spinner Mar 3, 2017
@@ -140,6 +140,8 @@
parser.pathname = parser.pathname + '/recent-updates';
$.ajax({
url: parser.href,
// Don't show plone default spinner
global: false,
Copy link
Member Author

@rodfersou rodfersou Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://api.jquery.com/jquery.ajax/

global (default: true)
Type: Boolean
Whether to trigger global Ajax event handlers for this request. The default is true. Set to
false to prevent the global handlers like ajaxStart or ajaxStop from being triggered. This
can be used to control various Ajax Events.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -95,6 +95,8 @@
parser.pathname = parser.pathname + '/recent-updates';
$.ajax({
url: parser.href,
// Don't show plone default spinner
global: false,
Copy link
Member Author

@rodfersou rodfersou Mar 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://api.jquery.com/jquery.ajax/

global (default: true)
Type: Boolean
Whether to trigger global Ajax event handlers for this request. The default is true. Set to
false to prevent the global handlers like ajaxStart or ajaxStop from being triggered. This
can be used to control various Ajax Events.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hvelarde
Copy link
Member

hvelarde commented Mar 6, 2017

I don't see any problems if the spinner is only duplicated during AJAX calls; I thinks it's even better to show it.

@hvelarde hvelarde closed this Mar 6, 2017
@hvelarde hvelarde deleted the issue-35 branch March 6, 2017 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spinner is duplicated in the liveblog
2 participants