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

Update: Make item titles optional, create fallback title for dialogs and strapline buttons (fixes #308) #309

Merged
merged 4 commits into from
Jul 2, 2024

Conversation

swashbuck
Copy link
Contributor

@swashbuck swashbuck commented Jun 25, 2024

Fix #308

Update

  • Make _items titles optional
  • Adds new generic fallback title _globals._components._narrative.titleDialog for strapline dialogs. This will only be used by screenreaders and will not be visually displayed. Defaults to Item {{itemNumber}} of {{totalItems}}.
  • Adds new generic fallback title _globals._components._narrative.titleStrapline for strapline buttons. Defaults to "Find out more"

Rework strapline dialog titles to use one of the following properties (sorted by precedence):

  1. _items title
  2. _items strapline
  3. _globals._components._narrative.titleDialog

Rework strapline button titles to use one of the following properties (sorted by precedence):

  1. _items strapline
  2. _items title
  3. _globals._components._narrative.titleStrapline

@swashbuck swashbuck self-assigned this Jun 25, 2024
@swashbuck swashbuck changed the title Fix: Make item titles option and create fallback title for dialogs (fixes #308) Fix: Make item titles optional and create fallback title for dialogs (fixes #308) Jun 25, 2024
Copy link
Contributor

@kirsty-hames kirsty-hames left a comment

Choose a reason for hiding this comment

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

The fallback title for notify dialog works as expected thanks @swashbuck.

However, having required title meant the strapline button always had a title. In the instance no title or strapline is set, .narrative__strapline-btn has no visual text or aria-label and the button collapses. See screen shot below.

no_strapline_or_title_set

To prevent this we could do one of the following:

  • provide a default strapline title e.g. "Find out more"
  • set titleDialog as strapline title.
  • set titleDialog as the strapline aria-label (no visual text) and prevent the button collapsing with a styling fix.

Thoughts on above or alternative solutions welcomed?

@swashbuck
Copy link
Contributor Author

swashbuck commented Jun 27, 2024

However, having required title meant the strapline button always had a title. In the instance no title or strapline is set, .narrative__strapline-btn has no visual text or aria-label and the button collapses.

To prevent this we could do one of the following:
* provide a default strapline title e.g. "Find out more"
* set titleDialog as strapline title.
* set titleDialog as the strapline aria-label (no visual text) and prevent the button collapsing with a styling fix.

@kirsty-hames I like the idea of having of a default strapline title (titleStrapline). It should then be obvious to course creators (in theory) that they need to set a "real" value for strapline if they do not want the generic fallback text.

This is another case where we need a better form library in the AAT. strapline should be a required field if the Narrative uses the strapline layout. Otherwise, it should not be required. The current form library doesn't have the capability of making fields conditionally required. But that's a matter for another day :)

@swashbuck swashbuck requested a review from kirsty-hames June 27, 2024 15:17
Copy link
Contributor

@kirsty-hames kirsty-hames left a comment

Choose a reason for hiding this comment

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

Works as expected thanks 👍

@swashbuck swashbuck changed the title Fix: Make item titles optional and create fallback title for dialogs (fixes #308) Fix: Make item titles optional and create fallback title for dialogs and strapline buttons (fixes #308) Jun 27, 2024
@swashbuck swashbuck changed the title Fix: Make item titles optional and create fallback title for dialogs and strapline buttons (fixes #308) Fix: Make item titles optional, create fallback title for dialogs and strapline buttons (fixes #308) Jun 27, 2024
@cahirodoherty-learningpool
Copy link
Contributor

Hi @swashbuck
I think the PR title should be changed to be an Update: ... type as new properties have been added

@swashbuck swashbuck changed the title Fix: Make item titles optional, create fallback title for dialogs and strapline buttons (fixes #308) Update: Make item titles optional, create fallback title for dialogs and strapline buttons (fixes #308) Jul 1, 2024
Copy link
Contributor

@joe-allen-89 joe-allen-89 left a comment

Choose a reason for hiding this comment

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

👍

@joe-allen-89 joe-allen-89 merged commit a13f7d3 into master Jul 2, 2024
@joe-allen-89 joe-allen-89 deleted the issue/308 branch July 2, 2024 10:57
github-actions bot pushed a commit that referenced this pull request Jul 2, 2024
# [7.10.0](v7.9.3...v7.10.0) (2024-07-02)

### Update

* Make item titles optional, create fallback title for dialogs and strapline buttons (fixes #308) (#309) ([a13f7d3](a13f7d3)), closes [#308](#308) [#309](#309)
Copy link

github-actions bot commented Jul 2, 2024

🎉 This PR is included in version 7.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revert item titles to optional
5 participants