Problem/Motivation

"Comments per page" setting cannot be configured to show "all comments" also if the dockbook of CommentStorageInterface::loadThread says:

   * @param int $comments_per_page
   *   (optional) The amount of comments to display per page.
   *   Defaults to 0, which means show all comments.

I can't set 0 for "Comments per page" due to hardcoded "min" on form element (introduced with #2239369: "Comments per page" setting cannot be configured to be larger than 300 and modified with #2906298: Comments per page number validation)

Proposed resolution

I think we could set "min" as 0 for permit to show all comments without pager.

In the next comment the patches for 8.7.x and 8.5.x

CommentFileSizeAuthor
#25 interdiff_20-25.txt575 bytesranjith_kumar_k_u
#25 2986372-25.patch3.44 KBranjith_kumar_k_u
#20 interdiff_16-20.txt1.6 KBranjith_kumar_k_u
#20 2986372-20.patch3.44 KBranjith_kumar_k_u
#19 interdiff_16-19.txt1.61 KBranjith_kumar_k_u
#19 2986372-19.patch3.45 KBranjith_kumar_k_u
#16 2986372-16.patch1.7 KBranjith_kumar_k_u
#16 interdiff_13-16.txt850 bytesranjith_kumar_k_u
#16 2986372-16-test-only.patch996 bytesranjith_kumar_k_u
#13 2986372-13.patch747 bytes_utsavsharma
#7 2986372.7.patch729 bytesranjith_kumar_k_u
#2 2986372-2-D8.5.x.patch750 bytesrobertom
#2 2986372-2-D8.7.x.patch727 bytesrobertom

Comments

robertom created an issue. See original summary.

robertom’s picture

robertom commented
Status: Active » Needs review
StatusFileSize
new 2986372-2-D8.7.x.patch727 bytes
new 2986372-2-D8.5.x.patch750 bytes

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
StatusFileSize
new 2986372.7.patch729 bytes

Re-rolled for 9.2

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Priority: Normal » Minor
Status: Needs review » Needs work
Issue tags: +Bug Smash Initiative, +Needs tests

Lets add a test for this, and review the UI text to make it clear that zero has a special meaning

_utsavsharma’s picture

_utsavsharma commented
StatusFileSize
new 2986372-13.patch747 bytes

Rerolled for 10.1.x.
Please review.

_utsavsharma’s picture

_utsavsharma commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Moving back for the tests.

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
StatusFileSize
new 2986372-16-test-only.patch996 bytes
new interdiff_13-16.txt850 bytes
new 2986372-16.patch1.7 KB

Added test, please review

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: -Needs tests

Good job expanding on an existing test case.

Think we need to also test that when it's set to 0 that all comments are appearing.

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
StatusFileSize
new 2986372-19.patch3.45 KB
new interdiff_16-19.txt1.61 KB

.

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
StatusFileSize
new 2986372-20.patch3.44 KB
new interdiff_16-20.txt1.6 KB

Added the tests as per #18

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Tests look good!

Good work

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented 
+++ b/core/modules/comment/src/Plugin/Field/FieldType/CommentItem.php
@@ -118,8 +118,9 @@ public function fieldSettingsForm(array $form, FormStateInterface $form_state) {
+      '#description' => $this->t('Enter 0 for show all comments.'),

Not a native English speaker, but shouldn't this be "Enter 0 for _showing_ all comments"?

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

I think it should probably be "Enter 0 to show all comments"

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
StatusFileSize
new 2986372-25.patch3.44 KB
new interdiff_20-25.txt575 bytes

Changed description text.

ranjith_kumar_k_u’s picture

ranjith_kumar_k_u commented
Status: Needs work » Needs review
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

That looks good.

  • catch committed 98217b5b on 10.1.x 
    Issue #2986372 by ranjith_kumar_k_u, robertom, _utsavsharma, smustgrave...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Looks good.

Committed 98217b5 and pushed to 10.1.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.