WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#28590 closed defect (bug) (fixed)

Post revision loading indicator( spinner ) wrapper div overlapping issue.

Reported by: vinod dalvi Owned by: SergeyBiryukov
Milestone: 4.0 Priority: normal
Severity: minor Version: 3.9.1
Component: Revisions Keywords: has-patch commit reporter-feedback
Focuses: ui Cc:
PR Number:

Description

The Post revision loading indicator( spinner ) overlaps on the other elements like buttons, menus etc. even when it is not active.

See the attached screenshot.

Patch added.

Attachments (4)

spinner.png (49.1 KB) - added by vinod dalvi 6 years ago.
Screenshot of the issue.
28590.diff (1.1 KB) - added by vinod dalvi 6 years ago.
Patch File.
28590-2.diff (563 bytes) - added by vinod dalvi 6 years ago.
Removed the patch edit from the file wp-admin/css/revisions-rtl.css as it's generated automatically.
28590.3.diff (645 bytes) - added by SergeyBiryukov 6 years ago.

Download all attachments as: .zip

Change History (23)

@vinod dalvi
6 years ago

Screenshot of the issue.

@vinod dalvi
6 years ago

Patch File.

#1 @vinod dalvi
6 years ago

  • Focuses ui added
  • Keywords has-patch added

@vinod dalvi
6 years ago

Removed the patch edit from the file wp-admin/css/revisions-rtl.css as it's generated automatically.

#2 @adamsilverstein
6 years ago

Vinod, thanks for taking the time to submit a patch! I'm not sure I see the same problem you encountered, in your screenshot the loading indicator is not visible!

Can you explain when you see the loading indicator? although I can see it its on the page HTML, its opacity is set to zero so it is NOT visible. I did some testing with a slow connection simulator and the loading indicator shows only until the revision views load.

The loading indicator is only supposed to show when the JavaScript has requested new data from the server and it is still loading, typically seen on a slower connection, if you try to view another revision after loading, or in the compare two mode. Are you seeing something different? If so can you give some more details about your browser version & test environment.

Thanks!

#3 @WPMUDEV-Support1
6 years ago

  • Summary changed from Post revision loading indicator( spinner ) overlapping issue. to Post revision loading indicator( spinner ) wrapper div overlapping issue.
Last edited 6 years ago by WPMUDEV-Support1 (previous) (diff)

#4 follow-up: @vinod dalvi
6 years ago

@adamsilverstein, Thank you for your reply. You don't need to make the loading indicator visible to reproduce the issue.

The issue is related with the spinner container div having class loading-indicator which overlaps on buttons, menus etc. even when the loading indicator is not visible and therefore you can't click on those elements.

You can't see this div having class loading-indicator as it is transparent so to reproduce this issue you can just apply temporary background color to it using browser elements inspector as displayed in the attached screenshot and vertically scroll the page so that it overlaps on any element.

I hope this description will help you to reproduce the issue but if you can't then don't hesitate to ask any questions.

#5 in reply to: ↑ 4 @adamsilverstein
6 years ago

  • Keywords needs-testing added

Thanks for clearing that up.

I misunderstood and thought you were saying the indicator was visibly obscuring these buttons, now I understand you mean its invisible and still blocks interaction with the underlying elements. Given that, you fix seems reasonable, however I'd like to dig a little more to make sure this is the best fix and to see if I can determine when the issue crept in.

Thanks gain for raising the ticket and providing a patch!

Replying to vinod dalvi:

@adamsilverstein, Thank you for your reply. You don't need to make the loading indicator visible to reproduce the issue.

The issue is related with the spinner container div having class loading-indicator which overlaps on buttons, menus etc. even when the loading indicator is not visible and therefore you can't click on those elements.

You can't see this div having class loading-indicator as it is transparent so to reproduce this issue you can just apply temporary background color to it using browser elements inspector as displayed in the attached screenshot and vertically scroll the page so that it overlaps on any element.

I hope this description will help you to reproduce the issue but if you can't then don't hesitate to ask any questions.

#6 @adamsilverstein
6 years ago

Seems like a reasonable solution, I did see some interactions breaking at certain screen sizes that are fixed by the patch.

Out of curiosity, what browser and screen size where you using when you noticed this problem?

#7 @adamsilverstein
6 years ago

  • Keywords needs-testing removed

#8 @adamsilverstein
6 years ago

  • Keywords dev-feedback added

#9 @adamsilverstein
6 years ago

  • Severity changed from normal to minor

#10 follow-up: @vinod dalvi
6 years ago

I noticed this problem on the latest version of Windows Chrome browser having Window size: 1366 x 728 and Viewport size: 1366 x 329 but i think this can be reproduced on any browser and on any screen size.

#11 in reply to: ↑ 10 @adamsilverstein
6 years ago

Replying to vinod dalvi:

I noticed this problem on the latest version of Windows Chrome browser having Window size: 1366 x 728 and Viewport size: 1366 x 329 but i think this can be reproduced on any browser and on any screen size.

Yes, its just surprising we never noticed the issue before, I think its because of your very short viewport height, still agree it should be fixed and your z-index approach does the trick.

This ticket was mentioned in IRC in #wordpress-dev by adamsilverstein. View the logs.


6 years ago

#13 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 4.0

#14 @SergeyBiryukov
6 years ago

  • Keywords commit added; dev-feedback removed
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

I think a better fix would be to properly position the spinner instead. See 28590.3.diff.

#15 @SergeyBiryukov
6 years ago

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

In 29054:

Revisions: Properly position the spinner on Compare Revisions screen.

props vinod dalvi for initial patch.
fixes #28590.

#16 follow-up: @vinod dalvi
6 years ago

@SergeyBiryukov, I have tested the patch you have added and it's working fine better than old one. The only issue i saw is on the mobile devices having smaller height the loading indicator goes outside the viewable area.

This can be fixed if we move loading indicator upwards by reducing its top property value.

#17 in reply to: ↑ 16 @SergeyBiryukov
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to vinod dalvi:

The only issue i saw is on the mobile devices having smaller height the loading indicator goes outside the viewable area.

Could you add a screenshot?

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


5 years ago

#19 @DrewAPicture
5 years ago

  • Keywords reporter-feedback added
  • Resolution set to fixed
  • Status changed from reopened to closed

Closing as fixed. Looks like we may still need a followup on the mobile experience.

Note: See TracTickets for help on using tickets.