Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hello.
In my case I need 5 comments per page. But I can't submit form because of this field have validation - number with minimum value = 10 and with step = 10.
It's really not clear to me why this "min" restriction was imposed. I also think that this will not be a pleasant thing for many users who want to change the number of comments per page for example to 15.
Steps to reproduce:
- Go to /admin/structure/types/manage/{CONTENT_TYPE}/fields/node.{CONTENT_TYPE}.comment
- In field "Comments per page" enter any number less than 10 or not divisible by 10
Comment | File | Size | Author |
---|---|---|---|
#3 | drupal-core-2906298-3.patch | 694 bytes | qzmenko |
#2 | drupal-core-2906298-2.patch | 674 bytes | qzmenko |
Comments
Comment #2
qzmenkoSo, I suggest removing these restrictions. I could not find the reason why they were added.
Comment #3
qzmenkoIt's necessary to leave the validation to a minimum value of 1. To prevent a value of 0 or less.
Comment #5
qzmenkoAny reviews?
Comment #6
runeasgar CreditAttribution: runeasgar as a volunteer commentedReviewing this now.
Comment #7
runeasgar CreditAttribution: runeasgar as a volunteer commented/admin/structure/types/manage/article/fields/node.article.comment
.$ curl -l https://www.drupal.org/files/issues/drupal-core-2906298-3.patch | git apply -v
.drush cr
, refresh.Looks good to me. Seeing as how this is not marked as a "bug", I'm presuming it doesn't need tests. Also, since it's not really a UI change, assuming it doesn't need a screenshot (and not sure what I'd screenshot, anyway).
Marking as RTBC. Thanks, @qzmenko !
Comment #9
qzmenkoI returned RTBC from #7, because after the re-testing it successfully passed.
Comment #10
alexpottDiscussed with @yoroy and we agreed that this is okay to go in. I would not recommend a setting of 1 or even 5 but that's not up to me - this is something that a site should be able to choose for whatever reason. Also since this is just changing FAPI structure no test is necessary.
Crediting @runeasgar for the thorough review in #7.
Comment #11
alexpottCommitted 7476b37 and pushed to 8.6.x. Thanks!