Problem/Motivation

In case of notifications remaining, revisedSentance text value on the itemsStatusChanged() differs by notification count.
The current implement use several Drupal.t() and if-else clause, finally concatenate these text but this section can write at once with Drupal.formatPlural().

Proposed resolution

Before:

          revisedSentance = `${Drupal.t(
            'You have ',
          )}<span class="bg-white px-1 text-black fw-bold rounded-1 fst-italic">${
            remainingCount
          }</span>`;
          if (remainingCount === 1) {
            revisedSentance += Drupal.t(' unread notification');
          } else {
            revisedSentance += Drupal.t(' unread notifications');
          }

After:

          revisedSentance =
           Drupal.formatPlural(
             remainingCount,
             'You have <span class="bg-white px-1 text-black fw-bold rounded-1 fst-italic">1</span> unread notification',
             'You have <span class="bg-white px-1 text-black fw-bold rounded-1 fst-italic">@count</span> unread notifications',
           );

Remaining tasks

User interface changes

API changes

Data model changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

tom konda created an issue. See original summary.

tom konda changed the visibility of the branch 3576338-multiple-drupal.t-in to hidden.

tom konda changed the visibility of the branch 3576338-multiple-drupal.t-in to active.

tom konda changed the visibility of the branch 3576338-multiple-drupal.t-in to hidden.

tom konda’s picture

Status: Active » Needs work
tom konda’s picture

Title: Multiple Drupal.t() in notifications.js can rewritten single Drupal.formatPlural() » Multiple Drupal.t() in notifications.js can be rewritten single Drupal.formatPlural()
Status: Needs work » Needs review
bramdriesen’s picture

I think this looks good. I just keep having "personal" issues with the span tag 😅 normally I try to keep HTML out of translatable strings. But I'm unsure how to solve it in this case.

tom konda’s picture

I checked existing CSS files on this module, there were no rules for the span element or it's class such as "bg-white", "px-1" and etc.
I don't know why the span tag was added on past commit, I think the span can be removed.

bramdriesen’s picture

Status: Needs review » Needs work

Sounds like a solution! I also can't remember the number from looking different in the widget. Maybe it was done for easier theming. But then that should be done in the twig template I guess.

tom konda’s picture

Title: Multiple Drupal.t() in notifications.js can be rewritten single Drupal.formatPlural() » Multiple Drupal.t() in notifications.js can be rewritten single Drupal.formatPlural() and be removed span
Status: Needs work » Needs review

I remove span tag and change issue title.
Please review.

bramdriesen’s picture

Status: Needs review » Fixed

Thanks Tom!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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