Add support for plural in Views Global result summary plugin

Based on
https://www.drupal.org/node/1793500

With the patch you be able to adjust it in the view settings as admin.

Hello team,

I needed to have plural on my results count in Views for 8.3.7 version of Drupal.

I had an error when applying the patch for 8.4 version , so I adapted it for 8.3.7.

Hi !

With the patch in https://www.drupal.org/node/2888320#comment-12303083 when there is 0 result it is in plural whereas it has to be singular IMO.

So here a new patch for Drupal 8.3.x with an interdiff.

We have the same issue with the patch in https://www.drupal.org/node/2888320#comment-12226674 so I'll update it sooner.

The is the new patch with the fix (https://www.drupal.org/node/2888320#comment-12340067) in 8.4.x version with the interdiff.

Thanks

+1 RTBC Works for me.
Also 0 gives the plural 'results'

Seems like a feature request more then a task. New features go into the development branch, 8.5.x at the moment.

Some feedback on the current patch:

1. +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -25,6 +25,12 @@ protected function defineOptions() {
   +    $options['format_plural'] = array('default' => FALSE, 'bool' => TRUE);
   +    $options['format_plural_count'] = array('default' => '@total');
   +    $options['format_plural_plural'] = array(
   
   @@ -47,6 +53,16 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
   +    $format_plural_count_options = array(
   

   all arrays need to be updated to the short [] notation

2. +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -55,6 +71,26 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
   +    $form['format_plural_count'] = [
   ...
   +    $form['format_plural_plural'] = [
   

   Would be nice to have some #states on these fields and only show them when 'format_plural' is checked

3. +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -112,6 +148,10 @@ public function render($empty = FALSE) {
   +    // If the format_plural option is selected.
   

   This comment is not very helpful

We'd need test coverage for this and an upgrade path for the new settings.

Fixed the following feedback given by @Lendude.
1 - Fixed all the arrays with short [] notation.
2 - Added drupal #states and only show relevant field when 'format_plural' is checked.
3 - Update the comment.

Does the patch support translation? I couldn't find any field to configure the plural translation.

I have added to views.area.schema.yml the plural field in order to get this same field translatable.

I removed a value (translatable => TRUE) from the defineOptions method which does not have any impact on the translatability of the field. If I am wrong, you can restore this line (check the interdiff for details).

I added a line break to a comment to respect code standard (phpcs).

We had a project where we had to show different messages for plural forms but we also had to cover the case where there are *0 results*. This patch did the trick for me.

+++ b/core/modules/views/src/Plugin/views/area/Result.php
@@ -55,6 +68,36 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
+      '#title' => t('Format plural'),
+      '#description' => t('If checked, special handling will be used for plurality.'),
...
+      '#title' => t('Count token'),
+      '#description' => t('Token used to select plural format.'),
...
+      '#description' => t('Text to use for the plural form.'),

Should be $this->t()

Needs work for tests, upgrade path & naming

1. +++ b/core/modules/views/config/schema/views.area.schema.yml
   @@ -50,6 +50,9 @@ views.area.result:
   +    format_plural_plural:
   
   +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -25,6 +25,9 @@ protected function defineOptions() {
   +    $options['format_plural_plural'] = ['default' => ''];
   
   @@ -55,6 +68,36 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
   +    $form['format_plural_plural'] = [
   +      '#title' => t('Plural form'),
   ...
   +      '#description' => t('Text to use for the plural form.'),
   

   Bad naming, suggestion is "format_plural_text"

2. +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -25,6 +25,9 @@ protected function defineOptions() {
   +    $options['format_plural'] = ['default' => FALSE, 'bool' => TRUE];
   +    $options['format_plural_count'] = ['default' => '@total'];
   

   This options also needs config schema

This patch worked great for me. I've attached a slightly updated version with feedback from andypost.

Attaching an updated patch of the one posted in #21 by MattDanger.

Every language has a different pluralization formula. It's not enough to just check if the number is more than 1 to decide which variant to show. Even for English this approach fails for zero results. In order to use the formulas, the updated patch uses PluralTranslatableMarkup::createFromTranslatedString(), as the view gets translated with the config translation.

• We tested latest patch from comment above and it works great : 
  • Tested with a multi langual website (EN default lang + multiples translations)
  • String displayed as expected accordingly to result count and active lang

Header UI in view

Single/plural string translation

@davidferlay thanks for the manual tests but we also need automated test coverage of new features. For more information about writing tests in Drupal 8 see the following links:

1. https://www.drupal.org/docs/8/testing
2. https://api.drupal.org/api/drupal/core%21core.api.php/group/testing/8.7.x

Also as per @Lendude we need an upgrade path.

Hello,

Here is a patch to provide automated tests. I have been helped by https://www.drupal.org/u/grimreaper.

But I don't find any existing test to guide me to test that the plugin is translatable.

I will try to search for an upgrade path.

Thanks for any help on the upgrade path and the translation tests.

Hello,

We tried to fix problems with test and we found a solution for an upgrade path

I put this in this new patch.

Thanks for the review.

Sorry,

I forgot to insert the original modifications. I re-upload the patch with all the changes.

I've tested the patch in #31 and it appears to work for plural formatting, but there doesn't seem to be any support for non result summary tokens. The issue title seems to imply that there was a desire to support other tokens but maybe I'm misunderstanding. Is there any reason this views area shouldn't be extending TokenizeAreaPluginBase instead and calling tokenizeValue() on the output?

return [
  '#markup' => $this->tokenizeValue($output),
];

Adding patch with token support in case it's useful.

Hello,

I have test your code and its works, but you have forgotten the views test. I add it. The test works now.

Thanks for the review.

Hello,

Here is an updated patch from comment 35.

I found that it was missing "tokenize" key in the config has now the Result plugin extends TokenizeAreaPluginBase.

Fixing schema.yml

Fixed Unit tests.

Tests are green \o/!!! Finally...

1. +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -55,6 +69,36 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
   +      '#description' => $this->t('If checked, special handling will be used for plurality.'),
   

   I think 'special handling' is too vague, can we get a clearer description of what this does?

2. +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -55,6 +69,36 @@ public function buildOptionsForm(&$form, FormStateInterface $form_state) {
   +      '#description' => $this->t('Token used to select plural format.'),
   

   This could be clearer too, reading this it's not immediately obvious to me how to use this.

3. +++ b/core/modules/views/views.install
   @@ -532,3 +532,44 @@ function views_update_8500() {
   +function views_update_8800() {
   +  $config_factory = \Drupal::configFactory();
   +
   +  foreach ($config_factory->listAll('views.view.') as $view_config_name) {
   

   Can we make this a post_update function and use ConfigEntityUpdater? See \views_post_update_limit_operator_defaults
   Also this still needs an update test.

Comment 48 taken into account.

Unfortunately I have the update test failing locally, the new update hook is not detected. I will see with testbot if it comes from my setup.

Also catched that the tokenize key was not inited with the previous update hook.

Rebasing patch.

Conflict in the views.post_update.php file.

And here is a new patch.

Not putting an interdiff because of the new strategy decided with Lendude => Creating a new dedicated plugin.

So it is a completely new patch.

Maybe the issue title should be changed.

Just to explain the reasoning behind the new plugin approach:
- way less disruptive
- no update paths needed
- Much much much clearer UI (the modal for the previous patches was pretty hard to read)

Will try to do a proper review soon

retagging

This looks great. Some nits:

1. +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -75,53 +89,60 @@ public function render($empty = FALSE) {
   +   * Helper function for easier reusable code.
   

   This needs to explain what it does not why it exists :)

2. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaResultPluralTest.php
   @@ -0,0 +1,77 @@
   +  /**
   +   * Tests the results plural area handler.
   +   */
   +  public function testResult() {
   ...
   +  /**
   +   * Tests the results plural area handler.
   +   */
   +  public function testResultEmpty() {
   

   Both test methods have the same doc bloc, maybe specify which part they are testing

3. +++ b/core/modules/views/views.views.inc
   @@ -97,6 +97,14 @@ function views_views_data() {
   +    'help' => t('Shows result summary, for example the items per page (Plural handled).'),
   

   Can we give a better explanation here then just (Plural handled)?
   "Shows result summary, for example the items per page, with support for having singular and plural text depending on the number of results."
   I don't know, something like that? Just to make it clearer what the difference is between the two plugins.

Also, can we get some fresh screenshot of the new plugin, the ones in #23 are no longer relevant but they provide a nice arguments for the new plugin when combined with the new screenshots I feel.

Hello,

Thanks @Lendude for the review :)

1: Done, hope it is ok.
2: Done
3: Done, your sentence is better, I just didn't know that it was ok to have long sentence.

And screenshots attached.

It would be nice if the default value for the plural actually used a different text to showcase the difference with the current plugin.

Suggestion singular form: Displaying 1 result.
Suggestion plural form: Displaying @count results.

Thanks @idebr for the review.

Marking as RTBC to get feedback from maintainer before implementing your suggestion.

Re-roll patch #60

Thanks for this great work. I have been using this patch for a long time now.
But sadly, it's not working anymore since 8.8.4 Drupal core, neither with 8.9.x.

Fixing failed tests.

Hello,

+++ b/core/modules/views/src/Plugin/views/area/ResultPlural.php
@@ -82,7 +82,7 @@ public function render($empty = FALSE) {
+        PoItem::DELIMITER,

Why making this change? (comment 67)

Hello,

Ok I got it. PluralTranslatableMarkup::DELIMITER was deprecated.

As there is no interdiff in comment 64, I tried to apply patch from comment 60 on 9.2.x which still applies, so I don't understand the comment from patch 64.

And I confirm that I obtain the same code as patch from comment 59. after updating the deprecated code.

So back to RTBC to have maintainers feedbacks.

+++ b/core/modules/views/src/Plugin/views/area/ResultPlural.php
@@ -0,0 +1,106 @@
+    $form['content_plural'] = [
+      '#type' => 'textarea',
+      '#title' => $this->t('Plural form'),
+      '#description' => $description,
+      '#default_value' => $this->options['content_plural'],
+      '#weight' => 10,
+      '#rows' => 3,
+    ];

This is super tricky but this is not how we should be doing this. The problem is that this needs to be translatable so we need to be mindful of multiple plural forms for different languages. See \Drupal\views\Plugin\views\field\NumericField::buildOptionsForm() for an example of the work we need to do here to support multiple plural forms.

With no opinion on the cleanliness of the code, patch #69 seems to be working on a Drupal 9.2.2 website using English & French languages (translated plural field follows language as expected).

Attaching a re-roll of #69 for 9.3.x. Still needs work for #73

Aha! The use of Drupal\views\Plugin\views\area\Xss was removed in #2569381: Drupal\views\Plugin\views\area\Result does an unnecessary XSS::adminFilter() . Now we have to restore it here.
Uploading a new patch for 9.3.x.

Have been using this patch at #78 for a while.

Would like to see addition of one more token that will number format the "total" token with a thousands separator.

Could be something like: @format_total

with a replacement like:

number_format($total, '.', ',')

Would that be okay to add within this issue and proposed patch?

The merge request at #81 applies patch #78 with the following additions

• Fix plural count token option for @label (was @name)
• Add new token @total_format -- the total records in the set (number formatted)
• Update all tests

test failures picked up problems with the naming of the new token and the formatting call.
Fixed those now, rebased and queued for testing again.

Some thoughts:

• Using the view label/name as a count token for plural seems odd. Maybe it shouldn't be an available option.
• Would like to add token variations for @format_start @format_end to go with @format_total if okay

edited wrong post

RE #78: it was removed for a reason, so it shouldn't be restored I think, but removed from this patch/MR as well?

We are still using the patch, so I re-rolled the patch to Drupal 10.1

Patch #90 works for me using Drupal 10.1.5

Reroll for 10.3.x

Hello, I'm upgrade my project with Drupal 10.3 but patchs (#78 and #90) are not compatible with them.

Can I fix the problem? Thanks

Patch #92 works for me with Drupal 10.3

Patch #92 works for me with Drupal 10.3

Patch #92 works for me with Drupal 10.4.0 as well ;-)

I've updated #92 to the new object oriented hook implementations in Drupal 11.1.

Patch #97 works for me using Drupal 11.1.2 😋

Patch #97 still works for me using Drupal 11.1.7 😋 while MR!2422 doesn't apply (patching using cweagans/composer-patches).

I created a new MR based on the latest patch.

What's the point of having a second ResultPlural class parallel to the existing one? I can't figure out what's different about it from reading the plugin docs anyway. Why not put all the functionality in the existing one?

Also, changing the issue title, as the IS and the MR don't do anything to add tokens.

What's the point of having a second ResultPlural class parallel to the existing one?

This is explained in #56.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I merged the latest 11.x.

Left comments on the MR.

Updated patch for D11.3

11.x-dev is the correct target version. You can open a 11.3.x branch for a version-specific MR if necessary

Patch #110 working for me using Drupal 11.3.2

The new @format_total token feels out of scope. Should it be separated into its own issue?

Uploading MR diff as patch (applies to 11.3.5)

I created a separate issue for the @format_total token: #3581884: Add new @format_total token to Views Global result summary plugin
And removed it from this MR.