Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non-broken fallback text when maxlength/maxwords is 0 #2915

Merged
merged 8 commits into from
Nov 3, 2022

Conversation

romaricpascal
Copy link
Member

Avoids displaying a "You can enter undefined characters" to users if neither maxlength nor maxwords is set.

This'll avoid services from getting an unsavory message in front of their users if they chose to use JavaScript to pass the maxlength or maxcount option. (Closes #2910).

Unfortunately, before JavaScript kicks in, we can't tell if it'll be characters or words (nor the limit obviously), so the message is pretty vague (but still better than having a message that looks like there was a bug). @claireashworth, if you have a better wording, I'm open to suggestions.

Given we're expecting JavaScript to kick in and set a dynamic text based on the content of the component, we do need to have an element on the page taking some vertical space to avoid layout shifts, so some content is required. And as we're passing the option as text,   or other kind of HTML entities will be escaped.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 13, 2022 10:57 Inactive
@querkmachine
Copy link
Member

I'm kinda torn about this.

I'm not sure whether providing messaging reading something like "You can enter a limited amount of content" is actually useful to an end user, either. To me it raises more questions ("if they know there's a limit, why am I not told it?"), in part because it seems intentional and doesn't appear broken.

If it still said "undefined characters" I (at least, as a more technical person) would understand that to be an error, and would potentially report it.

It strikes me as being a situation where saying less or nothing at all might be the option that invokes the least cognitive load. We in frontend land don't actually know if there's a limit being imposed, so it doesn't feel like we should say that there is one.

@claireashworth
Copy link
Contributor

I agree. If there's no maxlength or maxword defined is there a default limit or does it become unlimited? If there's a default then we can refer to that. Is having nothing defined done for a particular use case? If so, we could possibly reference that. Otherwise, I agree it would be better if there was no message.

@querkmachine
Copy link
Member

If there's no maxlength or maxword defined is there a default limit or does it become unlimited? If there's a default then we can refer to that.

If no maxlength or maxwords setting is defined then, on the frontend, the field allows for an unlimited amount of input. There may be a maximum length defined on the serverside validation, but we have no way of knowing if that's the case. We don't specify a default maxlength in our code.

Is having nothing defined done for a particular use case?

It's not an expected use. If a service needs a large input without the character count functionality, we would expect them to use the textarea component instead.

@claireashworth
Copy link
Contributor

In that case, no message sounds like the better option.

@romaricpascal
Copy link
Member Author

Cheers for your feedback both. I'm not set on having a message at all cost, but just thinking of having a tidier situations for end users in case JavaScript doesn't kick in.

Before it would have been an error indeed to have the component rendered and forgetting to pass a maxlength or maxwords to the macro. But with the addition of the JavaScript configuration, it does become a relevant scenario as people can chose to configure their component in JavaScript.

What wouldn't be a use case is for them not to configure it in either places (though thankfully, as described in the issue linked to the PR, the end user experience is not that bad, with no text being displayed).

I'll see if I can provide an empty text instead of that generic message, which I'd agree, doesn't provide any help.

@36degrees
Copy link
Contributor

I think an unfortunate side effect of the config work that we didn't foresee is that the Character Count JS can now receive config which isn't available to the template, which I think needs to be factored in here.

I'd agree with omitting the count message entirely if neither maxwords nor maxlength is set in the macro, but if the component is then initialised in JS with new CharacterCount($element, { maxlength: 200 }) it feels like that should still work – there just wouldn't be an existing 'fallback message' in that context, and the JS might need to allow for that?

@romaricpascal
Copy link
Member Author

romaricpascal commented Oct 13, 2022

That hint element does two nice little things for the component (besides serving as an anchor for the JavaScript, but we could easily default to the textarea being that anchor if there's no element):

  1. it reserves space on the page, which means that when JavaScript will kick in, there won't be a shift of content. I'm not sure how critical that is to maintain that, it would provide a better experience for the users if buttons don't shift around as JavaScript loads.
  2. it serves as aria-describedby for the textarea, letting users know that the length of the input is limited when they focus the field. We don't have a number until JavaScript kicks in and when JavaScript does kick in, we have no string to format to inject it. My understanding is that we set the aria-describedby to that element to avoid double announcements (based on what I gathered from the issues linked on Refactor character count to inject new element #2577. We could use try passing down the fallback hint down to JavaScript for use as a translation:
    • if no maxwords or maxlength is set, pass it as data-i18n.fallback-hint, JS picks it up and injects it into the element. We don't pluralise in Nunjucks so we could not worry about it there as well (and it'll likely be a large number anyway)
    • if maxwords or maxlength are set, interpolate like we do now and don't pass it to JS as it'll be on screen.

Feels like quite a bit of impact though, I may be missing a simpler route 😬

@querkmachine
Copy link
Member

I might be misunderstanding here, sorry.

The primary purpose of the fallback hint is so that guidance on character limits is still available in situations where JavaScript has been disabled or failed to load/initialise correctly. In that regard, anything that requires passing it to or processing it in JavaScript is going to fail that requirement.

@romaricpascal
Copy link
Member Author

No worries, I might be not explaining it super well as well 😊 .

The element does indeed act as the fallback visible text before JavaScript loads (I should have listed that as the first thing it does). And for that role, I agree with you, it doesn't make sense to be relying on JavaScript to inject any content.

However, the element also serves to provide guidance on the character limit for assistive technology, through being linked to the textarea via aria-describedby. If it's empty and we don't fill it in when the component is configured in JavaScript, users of services chosing that road for configuring their CharacterCount won't have an announcement that there's a limit to the number of characters when they focus the field, only announcements when they type and the number of remaining/over limit characters get updated.

That's for this scenario that I propose to pass the fallbackHintText (or fallbackHintDefault if none is set) as a data-i18n.... attribute so that the JavaScript can set that accessible description, replacing %{count} with whatever was configured at initialisation.

The lack of maxlength/maxwords in the macro options would trigger the template to switch from injecting fallbackHintText/fallbackHintDefault inside the HTML and instead pass it as an i18n message to be set at component initialisation.

@querkmachine
Copy link
Member

Ahh, I think I understand now.

It's not the way we would want service teams to be doing this (we'd expect one of maxlength or maxwords to be set in the Nunjucks in virtually all circumstances, for progressive enhancement as much as accessibility), but on the offchance that a JS configuration is present and a Nunjucks one is not, then populating the fallback dynamically makes sense. 👍

@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from e2235a1 to 509991d Compare October 17, 2022 14:20
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 17, 2022 14:21 Inactive
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from 509991d to 03be6cb Compare October 17, 2022 14:26
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 17, 2022 14:27 Inactive
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from 03be6cb to 1453b9e Compare October 17, 2022 14:27
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 17, 2022 14:28 Inactive
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from 1453b9e to df7c1b4 Compare October 17, 2022 14:30
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 17, 2022 14:30 Inactive
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from df7c1b4 to 9b90449 Compare October 17, 2022 14:33
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 17, 2022 14:33 Inactive
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from 9b90449 to c76ea0c Compare October 17, 2022 15:29
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 17, 2022 15:30 Inactive
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from c76ea0c to ea7854d Compare October 17, 2022 16:29
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 17, 2022 16:29 Inactive
@romaricpascal romaricpascal requested a review from a team October 17, 2022 17:04
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from ea7854d to 1a62c29 Compare October 18, 2022 13:36
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 18, 2022 13:36 Inactive
@romaricpascal romaricpascal requested a review from a team October 18, 2022 14:16
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from 20fff96 to c94278c Compare October 27, 2022 14:03
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 27, 2022 14:03 Inactive
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from c94278c to 9fb5639 Compare October 27, 2022 14:07
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 27, 2022 14:07 Inactive
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from 9fb5639 to 0d14ebc Compare October 27, 2022 14:20
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 27, 2022 14:20 Inactive
@romaricpascal
Copy link
Member Author

@36degrees All your comments should have been adressed in that last push 😄 Main changes, as discussed, were:

  • moving to not passing a text to JavaScript unless people provide an explicit fallback hint, as we cannot guess whether it'll be characters or words
  • providing a default translation for the fallbackHint's other (hadn't thought that this would throw indeed, good spot)
  • vertical space is now reserved in CSS rather than in the markup. Still with extra content, but a zero-width space rather than non-breaking one (that allow us to have it at all times, which is simpler).

@romaricpascal
Copy link
Member Author

How would we feel about renaming that option "textareaDescriptionText" (data-i18n.textarea-description, i18n.textareaDescription) so we don't miss that it provides the textarea description (and just mention that this description will be visible to users until the JavaScript component is initialised)?

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 October 27, 2022 14:44 Inactive
Avoid infering that users will count characters if no maximum nor fallback hint is provided and just set an empty description.
If users provide their own hint to the Nunjucks macro, it will still get forwarded as data attribute in the HTML and interpolated once JavaScript provides the maximum
Use a zero-width space at the end of the content to ensure there's content in the element so that the layout does not shift when JavaScript kicks in
@romaricpascal romaricpascal force-pushed the character-count-no-maxwords-maxlength branch from 5e49ed2 to 3b8c3e3 Compare November 2, 2022 12:23
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 November 2, 2022 12:24 Inactive
@36degrees
Copy link
Contributor

How would we feel about renaming that option "textareaDescriptionText" (data-i18n.textarea-description, i18n.textareaDescription) so we don't miss that it provides the textarea description (and just mention that this description will be visible to users until the JavaScript component is initialised)?

Sounds good to me.

Changes look good too – but if we're planning to rename then I'll hold off approving until that's done.

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are still quite a lot of places where we talk about a 'fallback message' or 'fallback hint' - suspect some of them are fine but think it might be worth updating at least $fallbackLimitMessage and the related comments in the component JS?

$ ag fallback src/govuk/components/character-count 
src/govuk/components/character-count/character-count.yaml
43:  description: Text describing the maximum number of characters you can enter, which is announced to screen readers. The text is displayed as a fallback if the character count JavaScript does not run. Depending on how you configure this component and what parameters you add, instances of `%{count}` are replaced by the value of `maxwords`. If not configured, it uses `maxlength`. By default, fallback text is provided in English.
89:  - name: with custom fallback text
90:    description: with no-js fallback text translated into Welsh
92:      name: custom-fallback
93:      id: custom-fallback
300:  - name: when neither maxlength/maxwords nor fallback hint are set

src/govuk/components/character-count/template.njk
14:  This means we can't compute a default fallback message (also used as the textarea accessible

src/govuk/components/character-count/character-count.test.js
34:    it('shows the fallback message', async () => {
58:      it('hides the fallback hint', async () => {
440:                'when neither maxlength/maxwords nor fallback hint are set'
609:        it('interpolates the fallback hint in data attributes with the maximum set in JavaScript', async () => {
610:          // This tests that any fallback hint provided through data-attributes

src/govuk/components/character-count/character-count.mjs
94:    // Read the fallback if necessary rather than have it set in the defaults
124:  var $fallbackLimitMessage = document.getElementById($textarea.id + '-info')
129:  if ($fallbackLimitMessage.textContent.match(/^\s*$/)) {
130:    $fallbackLimitMessage.textContent = this.i18n.t('textareaDescription', { count: this.maxLength })
133:  // Move the fallback count message to be immediately after the textarea
135:  $textarea.insertAdjacentElement('afterend', $fallbackLimitMessage)
143:  $fallbackLimitMessage.insertAdjacentElement('afterend', $screenReaderCountMessage)
146:  // fallback element for backwards compatibility as these may have been
149:  $visibleCountMessage.className = $fallbackLimitMessage.className
153:  $fallbackLimitMessage.insertAdjacentElement('afterend', $visibleCountMessage)
155:  // Hide the fallback limit message
156:  $fallbackLimitMessage.classList.add('govuk-visually-hidden')
386: * @property {PluralisedTranslation} [textareaDescription] - Fallback hint

src/govuk/components/character-count/template.test.js
228:  describe('with custom fallback text', () => {
229:    it('allows customisation of the fallback message', () => {
230:      const $ = render('character-count', examples['with custom fallback text'])
261:    describe('with fallback hint set', () => {
263:      // it needs to pass down any fallback hint to the JavaScript
265:      it('renders the fallback hint as a data attribute', () => {
268:        // Fallback hint is passed as data attribute
279:    describe('without fallback hint', () => {
280:      it('does not render a fallback hint data attribute', () => {
283:          examples['when neither maxlength/maxwords nor fallback hint are set']

@romaricpascal
Copy link
Member Author

Yup, that last commit just updated the full variable name to its new one. I'm starting a round of the rest of the code to check for further references to "fallback" 😄

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2915 November 3, 2022 15:15 Inactive
@romaricpascal
Copy link
Member Author

@36degrees I think I've shifted all references of "fallback" to "textarea description" when talking about the Character Count (appart from when saying that this description is a fallback when JS doesn't run) 🎉 I'll update the documentation draft as well 😄

@romaricpascal romaricpascal merged commit 41da3f7 into main Nov 3, 2022
@romaricpascal romaricpascal deleted the character-count-no-maxwords-maxlength branch November 3, 2022 17:11
@romaricpascal romaricpascal mentioned this pull request Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CharacterCount doesn't render or initialise properly if maxlength or maxwords is not passed to the macro
6 participants