WordPress.org

Make WordPress Core

Opened 9 days ago

Closed 5 days ago

Last modified 4 days ago

#48866 closed defect (bug) (fixed)

TwentyTwenty: Paginated comments don't work

Reported by: andraganescu Owned by: SergeyBiryukov
Milestone: 5.3.1 Priority: normal
Severity: normal Version: 5.3
Component: Bundled Theme Keywords: has-patch needs-testing has-dev-note
Focuses: javascript Cc:
PR Number:

Description (last modified by SergeyBiryukov)

While investigating #41390 I tried testing as described using the new default theme TwentyTwenty:

The "Other comment settings" section can be found under Settings > Discussion.

Steps to reproduce:

Break comments into pages with 3 top level comments per page and the last page displayed by default. > Comments should be displayed with the newer comments at the top of each page
Go to a post which already has more than, say, 10 comments
Navigate to the previous comments using the comment pagination

... and found that when clicking "Previous comments" nothing happened.

Simply switching themes to Twenty Nineteen solved the issue.

Attachments (2)

48866.diff (448 bytes) - added by mukesh27 8 days ago.
Patch.
48866.1.diff (977 bytes) - added by Hareesh Pillai 5 days ago.
Updated patch

Download all attachments as: .zip

Change History (14)

This ticket was mentioned in Slack in #themereview by joyously. View the logs.


9 days ago

#2 @joyously
9 days ago

  • Component changed from Themes to Bundled Theme
  • Summary changed from Paginated comments don't work in TwentyTwenty to TwentyTwenty: Paginated comments don't work

It appears to be jumping to the in-page anchor instead of navigating to the other page of comments. (caught by JS?)

#3 @acosmin
9 days ago

It's a JS "issue" regarding smoothScroll. You can test by commenting out this line: https://github.com/WordPress/twentytwenty/blob/master/assets/js/index.js#L745

A solution would be to add a || -1 !== element.href.search( '#comments' ) on this line:
https://github.com/WordPress/twentytwenty/blob/master/assets/js/index.js#L341

#4 @SergeyBiryukov
9 days ago

  • Description modified (diff)

#6 @mukesh27
8 days ago

  • Keywords has-patch needs-testing added; needs-patch removed

If we remove comments id from the parent DIV then it will fix the issue.

@mukesh27
8 days ago

Patch.

#7 @acosmin
8 days ago

@mukesh27 removing that makes for bad ux. When I click a post's comments link (eg: from the index page) I expect it to point me (scroll to) the post's comment section.

The solution is the JS I mentioned...

Last edited 8 days ago by acosmin (previous) (diff)

@Hareesh Pillai
5 days ago

Updated patch

#8 @Hareesh Pillai
5 days ago

  • Focuses javascript added
  • Version set to 5.3

New patch as per the above comments. Tested in my environment and works fine.
It would be nice if we milestone this to 5.3.1.

#9 @SergeyBiryukov
5 days ago

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

In 46824:

Twenty Twenty: Replace Smooth Scroll JS implementation with scroll-behavior CSS property.

The JS implementation had multiple issues and did not work as expected.

This change includes an accessibility enhancement by using prefers-reduced-motion: reduce media query property for users that don't want motion effects. For further explanation on this media query, see MDN documentation: https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-behavior#Accessibility_concerns

Props audrasjb, melchoyce, joostdevalk, Anlino, mauteri, sergiomdgomes, littlebigthing, williampatton, netweb, andraganescu, joyously, acosmin, mukesh27, hareesh-pillai.
Fixes #48763, #48551, #48866.

#10 @SergeyBiryukov
5 days ago

In 46825:

Twenty Twenty: Replace Smooth Scroll JS implementation with scroll-behavior CSS property.

The JS implementation had multiple issues and did not work as expected.

This change includes an accessibility enhancement by using prefers-reduced-motion: reduce media query property for users that don't want motion effects. For further explanation on this media query, see MDN documentation: https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-behavior#Accessibility_concerns

Props audrasjb, melchoyce, joostdevalk, Anlino, mauteri, sergiomdgomes, littlebigthing, williampatton, netweb, andraganescu, joyously, acosmin, mukesh27, hareesh-pillai.
Merges [46824] to the 5.3 branch.
Fixes #48763, #48551, #48866.

#11 @SergeyBiryukov
5 days ago

  • Milestone changed from Awaiting Review to 5.3.1
Note: See TracTickets for help on using tickets.