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

New: Allow course item title to be overridden (fixes #205) #208

Merged
merged 8 commits into from
Mar 28, 2024

Conversation

swashbuck
Copy link
Contributor

@swashbuck swashbuck commented Mar 8, 2024

Fixes #205

New

  • Allows course item title to be overridden. Should be backwards compatible as the new title field isn't required.

Update

  • Splits properties in README.md into separate sections for course.json, contentObjects.json, and then article/blocks/components.json. I think this helps clarify what goes into the specific type of JSON.

Testing

In course.json, add a title property like:

"_pageLevelProgress": {
  "title": "Alternate course title",
  "_isEnabled": true
}

@swashbuck swashbuck self-assigned this Mar 8, 2024
@swashbuck
Copy link
Contributor Author

swashbuck commented Mar 8, 2024

While the original issue was just for overriding the course title, it's just as easy (and possibly useful) to allow all titles to be overridden. For components, this could be used when the component title is really long and we want to use something much shorter for the PLP item.

@oliverfoster Before I do any further work (schemas, documentation), thoughts on this approach? Or should I just restrict this to course-level config?

@swashbuck
Copy link
Contributor Author

I went ahead and applied this at all levels: course, contentObject, article, block, and component. Ready for review.

@oliverfoster
Copy link
Member

oliverfoster commented Mar 19, 2024

We have displayTitle and title on each content part.

As far as I understand, displayTitle is for page display and title is already for plp, making an additional attribute for plp somewhat questionable.

What is the use-case when the two original attributes are insufficient?

@swashbuck
Copy link
Contributor Author

We have displayTitle and title on each content part.

As far as I understand, displayTitle is for page display and title is already for plp, making an additional attribute for plp somewhat questionable.

What is the use-case when the two original attributes are insufficient?

I believe you're right in that this is not needed. If I recall correctly, we had an issue where we wanted a Page Nav component's title in the PLP to be different from what's shown in the component. So, we changed the title to "Page navigation" and kept the displayTitle as "Well done." The accessibility testing firm flagged this as problematic since the screen reader read one title while sighted users saw a different one.

I'll pull this feature from contentObjects, articles, blocks, and components, leaving it only for course config.

@oliverfoster
Copy link
Member

oliverfoster commented Mar 19, 2024

That seems sensible.

The course model title is slightly different in that it also forms part of the browser document.title, not just the plp title.

https://github.com/adaptlearning/adapt-contrib-core/blob/b67304eff947d2f88d864297a3877aebdb7f1b41/js/router.js#L54-L65

Obviously, _pageLevelProgress.title will be supported everywhere as a consequence of the way the code works, but I don't think it's necessary to explicitly allow an override in the schemas and readme, except for course.

@swashbuck
Copy link
Contributor Author

That seems sensible.

The course model title is slightly different in that it also forms part of the browser document.title, not just the plp title.

https://github.com/adaptlearning/adapt-contrib-core/blob/b67304eff947d2f88d864297a3877aebdb7f1b41/js/router.js#L54-L65

Obviously, _pageLevelProgress.title will be supported everywhere as a consequence of the way the code works, but I don't think it's necessary to explicitly allow an override in the schemas and readme, except for course.

Yep, that's what I was thinking. Keep it as a "hidden" feature but don't explicitly call it out in the documentation or schemas.

@swashbuck swashbuck requested a review from oliverfoster March 19, 2024 18:57
@swashbuck swashbuck changed the title New: Allow item titles to be overridden at the course, article, block, or component level (fixes #205) New: Allow course item title to be overridden (fixes #205) Mar 20, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

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

@kirsty-hames kirsty-hames merged commit cd9adf0 into master Mar 28, 2024
@kirsty-hames kirsty-hames deleted the issue/205 branch March 28, 2024 16:21
github-actions bot pushed a commit that referenced this pull request Mar 28, 2024
# [7.5.0](v7.4.0...v7.5.0) (2024-03-28)

### New

* Allow course item title to be overridden (fixes #205) (#208) ([cd9adf0](cd9adf0)), closes [#205](#205) [#208](#208)
Copy link

🎉 This PR is included in version 7.5.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.

Alternate course item title
4 participants