Follow up for #731724: Convert comment settings into a field to make them work with CMI and non-node entities

Problem/Motivation

#731724: Convert comment settings into a field to make them work with CMI and non-node entities Moves comment to a field api field. We would love to get the threading and pager settings into the formatter, instead of the field settings, this would allow this to be used in views etc ie anywhere else display settings can be configured

Proposed resolution

Move threading and pager settings into formatter settings (from field settings).
Move comment_new_page_count and comment_num_new to the CommentManager service
Change arguments to comment_new_page_count to include passing the number-per-page and the threading mode as arguments instead of fetching them from the field.
Update formatter-output to include data-comment-per-page and data-comment-mode attributes. Send these attributes to CommentController::renderNewCommentsNodeLinks() allowing it call the comment_new_page_count function with the new arguments (note it is the only place this function is called).

Remaining tasks

Write the patch

User interface changes

Settings will be moved from field settings to formatter settings.

API changes

comment_new_page_count moved to manager service and new arguments.
Note CommentController::renderNewCommentsNodeLinks is the only place this function is called.

Related

Postponed on #2068331: Convert comment SQL queries to the Entity Query API and #1920046: Remove comment_entity_view and introduce second formatter for links

Comments

Title:Move comment threading and per page settings to Formatter settingsMove comment threading and per page settings to formatter settings

This change require to unify the output generation logic so formatter simply calls a function with parameters.
Also this change affects a way we generate a link to calculate a page for another formatter (links) so this a ONLY problem here

The number of page of the comment depends on both parameters so I think the Links formatter should get settings from "full" mode formatter and fallback to defaults when full is not CommentList

Another task here to implement node_title_list() theme-override to show the comment count

The number of page of the comment depends on both parameters so I think the Links formatter should get settings from "full" mode formatter and fallback to defaults when full is not CommentList

Looks like this workaround brokes standard drupal view modes
Possibly if this will be resolved in such way - should we provide a good docs and disabling some administration forms in core for comments?

Status:Postponed» Active

Assigned:Unassigned» larowlan

Working on this one too

Updated issue summary with discussions regarding CommentController::renderNewCommentsNodeLinks()

Issue tags:+API change

Tagging for the changes to comment_new_page_count

Issue summary:View changes

Updated issue summary.

Yep, CommentController:renderNewCommentsNodeLinks() should then be updated to either receive the number of comments per page (meaning that they'd be stored in the client-side, meaning that a malicious user could easily try to overload the server by manipulating this number) or the formatter ID with which that setting is associated. The latter seems to be the safer option.

I'd say the route's path should be updated from

path: '/comments/render_new_comments_node_links'

to:

path: '/comments/render_new_comments_node_links/{formatter}'

But what if views uses it as a formattter, we don't have a formatter id?

So how does Views do it then? If Views is not using the same render pipeline, then quite possibly it should provide a similar yet different route that accepts the view ID and display ID, and retrieves the settings from there?

Not sure I get the new idea about node-links but originally the was idea to implement second formatter to use for teaser to display links only.
Also logic to count page and query thread could be changed slightly in #2068331-30: Convert comment SQL queries to the Entity Query API

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

uis

From what I read in #1901110-6: Improve the UX for comment bundle pages and comment field settings from one of the screenshot has a 'Allow comments link' but that's removed from that issue as #1901110-13: Improve the UX for comment bundle pages and comment field settings mentioned this issue.

Reading here I don't read about hiding the 'Add new comment' link per display mode as #2141929: Comment link or form is added to print view mode. and #1166114-107: Manage Displays Search Results doesn't manage the display of the search results.

Am I wrong or can I try to fix for only D7 #2141929: Comment link or form is added to print view mode.?