Problem/Motivation

As I was working on #3373867: [regression] "Comments field is required" when creating content for types with a comment field configured as hidden, I realized there are several issues with the comment settings:

  1. When comment settings default value is set as hidden, the radio button loads without a default value. This is not recommend most of the time according to Nielsen Norman.
  2. When there are no comments, the hidden option is not shown. Shouldn't the "Closed" option be hidden since that option displays an empty list of comments without an option to submit new comments?

Proposed resolution

Always show the option selected by default so that it's visible to users which option has been selected. Also, hide the "Closed" option when there are not comments posted to the entity.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#15 3373938-15.patch911 bytesdrpldrp
#14 no-default-value-example.png63.15 KBdrpldrp
#10 3373938-10.patch3.72 KBlauriii
#6 Screenshot 2023-07-12 at 11.17.18 AM.png41.92 KBkeshavv
#5 Screenshot 2023-07-12 at 11.13.12 AM.png54.07 KBkeshavv
#5 Screenshot 2023-07-12 at 11.00.37 AM.png51.02 KBkeshavv
#5 Screenshot 2023-07-12 at 11.00.37 AM.png51.02 KBkeshavv
#4 interdiff.txt1.39 KBlauriii
#4 3373938-4.patch4.03 KBlauriii
#2 3373938-2.patch3.21 KBlauriii

Comments

lauriii created an issue. See original summary.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Active » Needs review
StatusFileSize
new 3373938-2.patch3.21 KB

Status: Needs review » Needs work

The last submitted patch, 2: 3373938-2.patch, failed testing. View results

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs work » Needs review
StatusFileSize
new 3373938-4.patch4.03 KB
new interdiff.txt1.39 KB
keshavv’s picture

keshavv commented
StatusFileSize
new Screenshot 2023-07-12 at 11.00.37 AM.png51.02 KB
new Screenshot 2023-07-12 at 11.00.37 AM.png51.02 KB
new Screenshot 2023-07-12 at 11.13.12 AM.png54.07 KB

Yes, The issue that you mentioned in the description is exist. It's showing the validation message that you need to select one option.
The patch that you provided is working perfectly. I think we can go wit it.

Before patch.
Before patch
Before patch

After patch.
After patch

keshavv’s picture

keshavv commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Screenshot 2023-07-12 at 11.17.18 AM.png41.92 KB
catch’s picture

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

I have one question - what happens if an entity has no comments + comments closed before this patch is applied, - won't the selected option be hidden then? We might need to to only hide the option if it doesn't match the current value, or.. something.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Reviewed & tested by the community

Before if you had selected hidden (for example because it's default value), it would not be shown in the UI. There would also be no way to select hidden when you have no comments because of 🤷‍♂️

After the patch, "Closed" will be displayed always when it's the current value (set in #default_value). However, if it's not the currently selected option, it's hidden when there are no comments because it doesn't make sense to have this state in that use case. Why would you display an empty comment section (without a way to comment) instead of hide it if there are no comments?

catch’s picture

catch
he/him
Primary language English
commented

That makes sense now, so we're fixing the issue I was worried about introducing here.

Also committed the issue this was spun out of.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new 3373938-10.patch3.72 KB

Rerolled #4 on top of #3373867: [regression] "Comments field is required" when creating content for types with a comment field configured as hidden.

  • catch committed f6800535 on 11.x 
    Issue #3373938 by lauriii, keshav.k: Comment settings are confusing
catch’s picture

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

Committed f680053 and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

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

drpldrp’s picture

drpldrp commented
StatusFileSize
new no-default-value-example.png63.15 KB

This change affects migration.

It can cause required comment field on node edit form to display no default value.

Attached example image is node edit form comment field for a migrated node that had comment=1 (Closed).

There's a type mistmatch between $element['status']['#default_value'] and CommentItemInterface::CLOSED.

Maybe just cast the prior to int, or it's probably okay to not use strict comparison !=.

// Only hide the option when it's not the currently selected option.
if ((int) $element['status']['#default_value'] !== CommentItemInterface::CLOSED) {
  $element['status'][CommentItemInterface::CLOSED]['#access'] = FALSE;
}
drpldrp’s picture

drpldrp commented
StatusFileSize
new 3373938-15.patch911 bytes
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

@drpldrp It would be helpful if you could file a new issue, and link it from here 🙏

drpldrp’s picture

drpldrp commented
Related issues: +#3485467: Comment settings Closed option unavailable despite current value is Closed