You can configure for each contenttype how many comments shall be displayed per page. The array of options is hardcoded into comment.module. We had several times the use case that we wanted a number of comments that wasn't in the available choices. You can change these options using hook_form_alter() but I think it would be better to transfer this hardcoded array into config.

Files: 
CommentFileSizeAuthor
#3 1830966.png61.77 KBpenyaskito
#1 drupal-make-comment-per-page-options-config-1830966-1.patch891 bytesleschekfm
PASSED: [[SimpleTest]]: [MySQL] 46,840 pass(es). View

Comments

leschekfm’s picture

Issue tags: +Configuration system
FileSize
891 bytes
PASSED: [[SimpleTest]]: [MySQL] 46,840 pass(es). View

The attached patch allows to change these options.

Please review

leschekfm’s picture

Status: Active » Needs review
penyaskito’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
61.77 KB

Tested it and looks fine.

1830966.png

larowlan’s picture

catch’s picture

Status: Reviewed & tested by the community » Needs review

I don't understand why hook_form_alter() isn't sufficient here.

leschekfm’s picture

In my opinion these options were randomly chosen by the original developer, because there is no system that would explain why exactly these were chosen. Therefore I think this is hardcoded configuration at the moment and should be converted to the config system.

Another reason to convert this would be that is much more easy to edit only a config file than to create a new module just to override some settings.

This issue could also be bypassed altogether by replacing the select box with a normal text input.

heyrocker’s picture

I don't think hardcoded config has any place in the config system, it is only user-editable config that makes sense there. I agree with catch that this could just as easily be modified via a form alter. If it became a text input field then configuration would make more sense.

leschekfm’s picture

I don't think hardcoded config has any place in the config system, it is only user-editable config that makes sense there.

That's my point ;) I think this should be user configurable. I would compare this to the options you can define for a list field, which is clearly config. Maybe your concern is that there is no UI for editing the list of comment options at the moment?

If it became a text input field then configuration would make more sense.

Could you explain that? If the input element would be converted to a text field there wouldn't be a need to store any config for the input element as a text field has no predefined list of options.

Xen’s picture

@leschekfm

Well, it's sorta meta-configuration. If it should make sense to have it as config, there should also be a admin page to configure it. Having an admin setting to set the possible options in another admin setting is clearly overkill. So while I can see why it would be handy, it is a bit of misuse of the configuration system.

I think the argument should really be about whether we should grit our teeth and carry on with using form_alter, change the setting to a textfield (not UX sexy), or create a new fancy widget that combines the dropdown with a textfield (much like the windows combo field), if someone steps up to implement that.

penyaskito’s picture

Issue tags: +Needs usability review

I don't think this feature can make it for 8.x unless someone from UX marks it as an UX bug, tagging for usability review.

andypost’s picture

Status: Needs review » Needs work

This settings should be moved to formatter with sane defaults from field instance settings

andypost’s picture

Status: Needs work » Closed (duplicate)