Make Character Count use hint component for message and allow custom … #1650

LBHELewis · 2019-11-18T19:52:00Z

…classes to be added

Character count was using the hint markup but not the component for its count message. This PR changes that to use the hint component. It doesn't allow custom text or id as these are referred to in the javascript, but it does allow custom classes to be added for styling.

Relevant issue: #1645

36degrees · 2019-11-19T11:58:00Z

The changelog entry in this pull request will need updating as v3.4.0 has now been released.

LBHELewis · 2019-11-20T09:13:18Z

@36degrees Thank you - that's updated now.

NickColley

This is looking good, the new API option countMessageClasses aligns with the BEM class...

There's a few minor changes needed before we can get this in, if you don't have time let us know and we can try to finish this up when we do. :)

NickColley · 2019-11-25T11:07:18Z

CHANGELOG.md

+
+You can now add custom classes to the count message in the character count component, which has been changed to use the hint component rather than custom markup.
+
+- [Pull request #:1650: Make Character Count use hint component for message and allow custom classes to be added](https://github.com/alphagov/govuk-frontend/pull/1650)


Tiny style thing: "Pull request #:1650" -> "Pull request: #1650"

NickColley · 2019-11-25T11:09:06Z

CHANGELOG.md

+
+#### Add custom classes to the count message in the character count component
+
+You can now add custom classes to the count message in the character count component, which has been changed to use the hint component rather than custom markup.


When writing CHANGELOG notes we try to focus on what the user needs to know, so how we fixed or updating something is not required to understand the new feature.

So this could instead focus on how to use the new countMessageClasses option rather than the internal details on how we implemented this change.

NickColley · 2019-11-25T11:11:23Z

src/govuk/components/character-count/template.njk

-  <span id="{{ params.id }}-info" class="govuk-hint govuk-character-count__message" aria-live="polite">
-    You can enter up to {{ params.maxlength or params.maxwords }} {{'words' if params.maxwords else 'characters' }}
-  </span>
+  {{


Tiny style thing: We keep the macro on the same line as the curly

{{ govukHint({ ... }) }}

vs

{{ govukHint({ ... }) }}

NickColley · 2019-11-25T11:12:07Z

src/govuk/components/character-count/character-count.yaml

@@ -141,3 +145,12 @@ examples:
      threshold: 75
      label:
        text: Full address
+
+  - name: with count message class


Where possible we try to add tests rather than review application demonstrations for non-visible functionality. So we remove this example and test this with Jest rather than having to check the review application.

NickColley · 2019-12-09T14:13:10Z

src/govuk/components/character-count/character-count.yaml

@@ -59,6 +59,10 @@ params:
  type: object
  required: false
  description: HTML attributes (for example data attributes) to add to the textarea.
+- name: countMessageClasses


For other reviewers, should this be countMessage or message with a nested classes option?

Could allow for updates without breaking changes in the future.

{{ govukCharacterCount({ label: { text: "Can you provide more detail?" }, countMessageClasses: 'app-extra-class' }) }}

vs

{{ govukCharacterCount({ label: { text: "Can you provide more detail?" }, countMessage: { classes: 'app-extra-class' } }) }}

I think I'd tend towards the nested object, as it's likely we'll need to make the text customisable in the future (somehow) so that it can be overridden if you're using a different language.

36degrees · 2019-12-10T13:39:37Z

src/govuk/components/character-count/template.test.js

        hint: {
          text: 'It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’.'
-        }
+        },
+        countMessageClasses: 'app-extra-class'


I think this should probably be tested in a separate assertion – this test is looking at whether a separate actual 'hint' can be added to the character count ("'It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’.'") whereas this class is added to the count message, which is unrelated to the rest of the test. I'd also assert that the class is on the element, rather than using a snapshot for it.

36degrees · 2019-12-10T13:40:58Z

src/govuk/components/character-count/template.njk

+    id: params.id + '-info',
+    classes: 'govuk-character-count__message' + (' ' + params.countMessageClasses if params.countMessageClasses),
+    attributes: {
+      'aria-live': 'polite'


I know this isn't 'new', but we're changing the implementation and it doesn't look like there's a test checking that this attribute is set correctly. Given how important this is for assistive tech, could we add one?

36degrees · 2019-12-10T13:44:14Z

src/govuk/components/character-count/character-count.yaml

@@ -59,6 +59,10 @@ params:
  type: object
  required: false
  description: HTML attributes (for example data attributes) to add to the textarea.
+- name: countMessageClasses


I think I'd tend towards the nested object, as it's likely we'll need to make the text customisable in the future (somehow) so that it can be overridden if you're using a different language.

36degrees · 2019-12-10T13:45:52Z

src/govuk/components/character-count/template.njk

-  <span id="{{ params.id }}-info" class="govuk-hint govuk-character-count__message" aria-live="polite">
-    You can enter up to {{ params.maxlength or params.maxwords }} {{'words' if params.maxwords else 'characters' }}
-  </span>
+  {{ govukHint({


I am still a little unsure about this change – it generally feels a bit strange to me that we're using the hint component at all here (as opposed to just having a 'character count message' which happens to look a bit like a hint…) – however, I don't think this makes it any harder for us to change this later, so I think it's OK for now.

Update the tests so there is coverage for the count message functionality and include new functionality.

NickColley · 2019-12-10T15:13:02Z

@36degrees Updated the tests so there is coverage for the count message functionality and include new functionality.

Stale

LBHELewis mentioned this pull request Nov 18, 2019

Make character count use hint component #1645

Closed

NickColley added the awaiting triage Needs triaging by team label Nov 20, 2019

kellylee-gds removed the awaiting triage Needs triaging by team label Nov 20, 2019

kellylee-gds added this to Needs review in Design System Sprint Board via automation Nov 20, 2019

NickColley previously requested changes Nov 25, 2019

View reviewed changes

Design System Sprint Board automation moved this from Needs review to In progress Nov 25, 2019

kellylee-gds moved this from In progress to Needs review in Design System Sprint Board Nov 26, 2019

NickColley force-pushed the feature/make-character-count-use-hint branch 3 times, most recently from b16caec to a7c2346 Compare December 9, 2019 14:11

NickColley reviewed Dec 9, 2019

View reviewed changes

NickColley added blocked and removed blocked labels Dec 9, 2019

NickColley requested a review from m-green December 9, 2019 14:16

NickColley added this to the Next milestone Dec 9, 2019

36degrees reviewed Dec 10, 2019

View reviewed changes

emmalewis and others added 3 commits December 10, 2019 15:09

Allow custom classes for character count message

7cfeded

Update and refactor tests

9ddb5bd

Update the tests so there is coverage for the count message functionality and include new functionality.

Update CHANGELOG

6abc78b

NickColley force-pushed the feature/make-character-count-use-hint branch from a7c2346 to 6abc78b Compare December 10, 2019 15:10

36degrees approved these changes Dec 10, 2019

View reviewed changes

NickColley merged commit cfca85f into alphagov:master Dec 10, 2019

Design System Sprint Board automation moved this from Needs review to Done Dec 10, 2019

36degrees mentioned this pull request Jan 21, 2020

Release v3.5.0 #1706

Merged

snyk-bot mentioned this pull request Feb 25, 2020

[Snyk] Upgrade govuk-frontend from 3.0.0 to 3.5.0 ministryofjustice/offender-categorisation#400

Closed

snyk-bot mentioned this pull request Mar 10, 2020

[Snyk] Upgrade govuk-frontend from 3.4.0 to 3.5.0 ministryofjustice/sentence-planning#234

Merged

snyk-bot mentioned this pull request Mar 29, 2020

[Snyk] Upgrade govuk-frontend from 3.5.0 to 3.6.0 hmcts/vh-video-web#471

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Character Count use hint component for message and allow custom … #1650

Make Character Count use hint component for message and allow custom … #1650

LBHELewis commented Nov 18, 2019

36degrees commented Nov 19, 2019

LBHELewis commented Nov 20, 2019

NickColley left a comment •

edited

NickColley Nov 25, 2019

NickColley Nov 25, 2019

NickColley Nov 25, 2019

NickColley Nov 25, 2019

NickColley Dec 9, 2019 •

edited

36degrees Dec 10, 2019

36degrees Dec 10, 2019

36degrees Dec 10, 2019

36degrees Dec 10, 2019

36degrees Dec 10, 2019

NickColley commented Dec 10, 2019


		You can now add custom classes to the count message in the character count component, which has been changed to use the hint component rather than custom markup.

		- [Pull request #:1650: Make Character Count use hint component for message and allow custom classes to be added](https://github.com/alphagov/govuk-frontend/pull/1650)


		#### Add custom classes to the count message in the character count component

		You can now add custom classes to the count message in the character count component, which has been changed to use the hint component rather than custom markup.

Make Character Count use hint component for message and allow custom … #1650

Make Character Count use hint component for message and allow custom … #1650

Conversation

LBHELewis commented Nov 18, 2019

36degrees commented Nov 19, 2019

LBHELewis commented Nov 20, 2019

NickColley left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickColley Dec 9, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickColley commented Dec 10, 2019

NickColley left a comment •

edited

NickColley Dec 9, 2019 •

edited