Add notifications banner template macro and tests

[hannalaakso]

This PR adds

• a template macro for the notification banner component
• tests for the template logic
• examples of the component

Please don't review the stylesheets for now, they are from the demo version and need to be worked on separately; this could be done as part of #1934.

Addresses parts of #1935

[36degrees]

Looking really good so far 👍🏻

[36degrees]

We should probably update these to point to the Frontend docs at some point…

[hannalaakso]

Thanks for the reviews @36degrees and @vanitabarrett 🙌 I think I've now resolved most of the comments, mainly apart from #1977 (comment) where it'd be good to decide how we name space boolean data attributes. I've left 2 TO DO's in the template code pending a decision.

I'm going to do a review of the documentation side of things with Eoin next.

[EoinShaughnessy]

Does "text to use" suggest that users do something with the text once it's inside the notification banner? Suggested tweak: "The text to display within the notification banner."

Also, should this info display at the start of the description, i.e., after Required? This would inform the user what the text option is before they learn about the effect html has on it.

Re first and last sentences — could personalise and phrase in active voice. So, "If you set 'html', this..." and "If you provide 'html', this..."

[hannalaakso]

We're going to do a separate wider review of the template options docs as this comment touches on most of the components.

[EoinShaughnessy]

Could personalise first and last sentences and phrase them in the active voice: "If you set 'text', this..." and "If you provide 'html', this..."

Do we say 'parameter' instead of 'argument'?

Should "HTML to use within the notification banner" display at the start of the description, i.e., after Required? This would inform the user what the option is before they learn anything else about it.

[hannalaakso]

Do we say 'parameter' instead of 'argument'?

We actually say 'option' 😅 but this might not always be consistent throughout the rest of the tech docs.

[EoinShaughnessy]

Do the table options display in order of importance? If not, then is there a case for displaying them alphabetically? (I understand, though, if we wouldn't want to change the order because it would make this doc inconsistent with other docs.)

[hannalaakso]

We have a loose order of importance (very loose in fact) and we seem to have settled on a bit of a hierarchy where text/html come first and the classes option is always last. We could definitely review that though.

[EoinShaughnessy]

So this option is for the title text in the banner, and the text option (above) is for non-title text in the banner? In the table, should this option display above the text option, to reflect their order in the banner?

[hannalaakso]

Good question. We've tended to have the text/html options first as those could be considered to be the main content. In this particular case, title text is also not required (a placeholder text is used if you don't supply one) whereas text/html are.

[EoinShaughnessy]

Re first sentence — could just say "The title of the notification banner" (if that sounds ok).

What do we mean by “Defaults to ‘Important’” and ‘“‘Success’ for success type and ‘Error’ for error type’”? Also, is “‘Success’ for success type and ‘Error’ for error type’” self-evident (success for success, error for error)?

I think we can delete the parentheses as the style guide says not to use them.

Re last sentence — we could personalise this and make it active, for example, "If you provide 'titleHTML,' the 'title' argument..." (The options above this say "provide" instead of "supply," so I'd use the same wording here.)

[hannalaakso]

What do we mean by “Defaults to ‘Important’” and ‘“‘Success’ for success type and ‘Error’ for error type’”?

I agree, this is definitely not clear, will be good to chat about it next week.

[EoinShaughnessy]

Could we personalise and replace the passive? So: “If you provide 'titleHtml',…”

[EoinShaughnessy]

Mark's suggestion: "Sets the heading level of the title. The minimum is 1 and the maximum is 6. The default is 2."

How do we feel about beginning each description with a verb, like in Mark's suggestion? Would it be a good way to communicate the purpose of each option?

[EoinShaughnessy]

The description states what happens when you set type. Do we need to specify what type actually is, though?

Is this option to denote whether the notification banner is purely informational or else an alert box which the screen reader picks up on immediately?

Typo to remove: second "moves" in "Javascript moves the keyboard focus moves to the notification banner..."

Could personalise and make active the second last and last sentences. So, "If you set type to success..." and "If you do not set type..."

I'd also suggest changing "using aria-labelledby" to "uses aria-labelledby." This would be to parallel the active voice used in the previous clause ("...the notification banner defaults...").

[EoinShaughnessy]

The description for the type option (above) mentions the role attribute, but doesn’t explain it. Would it help users if this role description displayed before they read about the type option?

“If type is set to success or error, defaults to alert.” It’s the role that defaults (and not the type), is that right? If so, should we explicitly call that out?

Replace passive in last sentence? So: “If you set type to…”

[EoinShaughnessy]

The description for the type option (above) mentions the role attribute, but doesn’t explain it. Would it help users if this role description displayed before they read about the type option?

“If type is set to success or error, defaults to alert.” It’s the role that defaults (and not the type), is that right? If so, should we explicitly call that out?

Replace passive in last sentence? So: “If you set type to…”

[EoinShaughnessy]

The description for the type option above mentions the tabindex attribute, but doesn’t explain it. Would it help users if this tabindex description displayed before they read about the type option?

Are options set by default unless we state otherwise? If so, do we need to call this out somewhere?

Instead of "Not set by default," could we reword the first sentence to "Overrides the value that the browser sets for the tabindex attribute" (or similar)?

“If type is set to success or error, defaults to alert.” Is it the tabindex or the type that defaults?

Suggested tweak for last sentence: “If you set type to success or error, tabindex defaults to -1. To remove the attribute, set tabindex to false.”

[EoinShaughnessy]

Could break up the second sentence. So: “...on the notification banner. This provides information…”

Should this display first? “id used by the aria-labelledby attribute on the notification banner.” It defines the option, which might be the first info a user wants to know.

Replace passive? So, “if you set role to region” instead of “if role is set to region” and “If you set type…" instead of "If type is set..."

[EoinShaughnessy]

Is it necessary to say "Defaults to false"? Maybe we could emphasise what setting it to true does. So, "If you set initialFocus to true, the keyboard focus moves to the notification banner when the page loads" (or similar).

Could make last sentence active: "If you set type,…"

[EoinShaughnessy]

I wonder could we personalise this (e.g. "Classes you want to add to the notification banner").

[EoinShaughnessy]

Could edit this to remove parentheses (as per style guide advice) and personalise it. So, "HTML attributes that you want to add to the notification banner, for example, data attributes."

[hannalaakso]

Thanks @EoinShaughnessy for the super useful comments! 🙏 As discussed, we're going to incorporate your changes in a new pull request in order to unblock this PR as this is now blocking another PR from being merged.