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

Fix: Resolve title/altTitle discrepancy (fixes #535) #551

Merged
merged 14 commits into from
Jun 10, 2024
Merged

Conversation

swashbuck
Copy link
Contributor

@swashbuck swashbuck commented Jun 5, 2024

Fixes #535

Fix

  • Rework how _feedback.title and _feedback.altTitle work. _feedback.altTitle, if set, will take precedence and the feedback title will be visually hidden.
  • Removes altTitle from feedbackConfig() as it did not seem to be used in the Notify templates.

The new behavior is as follows:

  1. If _feedback.altTitle is set, this will be used as a title aria label. No visual title is displayed.
  2. If _feedback.title is set and _feedback.altTitle is not set, a title is displayed with this value.
  3. If neither _feedback.title nor _feedback.altTitle are set, a title is displayed with the component's title.
  4. If none of the above are set, _globals._accessibility.altFeedbackTitle is used as an aria label and no visual title is displayed.
  5. Otherwise, no title is displayed or used as an aria label

The precedence basically goes:

  1. _feedback.altTitle
  2. _feedback.title
  3. Component title
  4. _globals._accessibility.altFeedbackTitle

Related

This removes the _feedback.altTitle from the sample FW course. Otherwise, by default, the sample course does not show a feedback title due to the presence of the alt titles.
adaptlearning/adapt_framework#3567

@swashbuck swashbuck added the bug label Jun 5, 2024
@swashbuck swashbuck self-assigned this Jun 5, 2024
js/models/questionModel.js Outdated Show resolved Hide resolved
swashbuck and others added 2 commits June 6, 2024 08:55
Co-authored-by: Oliver Foster <oliver.foster@kineo.com>
@swashbuck swashbuck marked this pull request as ready for review June 6, 2024 15:03
@swashbuck swashbuck requested a review from oliverfoster June 6, 2024 16:03
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.

This is working as described thanks 👍

Copy link
Member

@oliverfoster oliverfoster left a comment

Choose a reason for hiding this comment

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

👀 this seems to make sense. I think...

If an altTitle is set in the feedback object, then only the altTitle will be used as an aria label.
If no altTitle is set in the feedback object, then the course will visually display the feedback object title or the component title or use the global altFeedbackTitle as an aria label.

js/models/questionModel.js Outdated Show resolved Hide resolved
Co-authored-by: Oliver Foster <oliver.foster@kineo.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@swashbuck swashbuck merged commit c556847 into master Jun 10, 2024
@swashbuck swashbuck deleted the issue/535 branch June 10, 2024 15:31
@oliverfoster
Copy link
Member

🎉 This PR is included in version 6.50.2 🎉

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.

Missing titles for feedback Notify popups
4 participants