Skip to content

feat: add custom html templates for components - #30108

Merged
pkulkark merged 1 commit into
openedx:masterfrom
open-craft:keith/bb_5583_themed_html_templates
Jul 20, 2022
Merged

feat: add custom html templates for components#30108
pkulkark merged 1 commit into
openedx:masterfrom
open-craft:keith/bb_5583_themed_html_templates

Conversation

@keithgg

@keithgg keithgg commented Mar 23, 2022

Copy link
Copy Markdown
Contributor

Description

At Opencraft, we have clients that want to be able to add custom HTML templates that show up while creating course components in Studio.

Currently, there's no way to achieve this besides forking edx-platform and adding the yaml files to common/lib/xmodule/xmodule/templates/

This PR allows operators to set a custom directory where templates can be additionally loaded from.

  • Checks if settings.CUSTOM_RESOURCE_TEMPLATES_DIRECTORY exists.
  • If so, load the yaml files with the same logic as in the xmodule/templates directory.

Supporting information

Testing instructions

  • Update the setting settings.CUSTOM_RESOURCE_TEMPLATES_DIRECTORY to a valid path.
  • In that directory create an html directory.
  • Copy a template from common/lib/xmodule/xmodule/templates/html to the above directory with a different filename.
  • In the Studio, edit a Unit. Under the Text component you should see your new template list (on the Sandbox I've added many).

Deadline

"None"

Other

Settings

EDXAPP_CUSTOM_RESOURCE_TEMPLATES_DIRECTORY: /edx/var/edx-themes/edx-themes/resource-templates
edx_platform_commit: keith/bb_5583_themed_html_templates
edx_platform_repository_url: https://github.com/open-craft/edx-platform.git
configuration_source_repo_url: https://github.com/open-craft/configuration.git
configuration_version: keith/bb_5583_themed_html_templates

@openedx-webhooks

openedx-webhooks commented Mar 23, 2022

Copy link
Copy Markdown

Thanks for the pull request, @keithgg! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 23, 2022
@keithgg
keithgg force-pushed the keith/bb_5583_themed_html_templates branch from bb54ee8 to afe9048 Compare March 23, 2022 12:16
@natabene

Copy link
Copy Markdown
Contributor

@keithgg Thank you for your contribution. Please let me know once it is ready for our review.

@keithgg keithgg changed the title feat: add custom html templates for components in theme feat: add custom html templates for components Mar 25, 2022
@keithgg
keithgg marked this pull request as ready for review March 28, 2022 13:21
@openedx-webhooks openedx-webhooks added needs triage waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Mar 28, 2022
@keithgg

keithgg commented Mar 28, 2022

Copy link
Copy Markdown
Contributor Author

@natabene this is ready for review.

@navinkarkera

navinkarkera commented Mar 28, 2022

Copy link
Copy Markdown
Contributor

👍

  • I tested this: (followed steps mentioned in test instructions)
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation

@giovannicimolin giovannicimolin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@keithgg It's working perfectly for me. I left a few comments on the code, but mostly as suggestions to start a discussion.

Also, this needs a bit more documentation somewhere (maybe in the class description? and simple tips like file needs to be yaml, not yml, file structure needs to match the block type, etc).

Comment thread cms/envs/common.py Outdated
Comment thread lms/envs/common.py Outdated
Comment thread common/lib/xmodule/xmodule/x_module.py Outdated
@keithgg
keithgg force-pushed the keith/bb_5583_themed_html_templates branch 3 times, most recently from cbb4e60 to b54c79e Compare July 7, 2022 07:28
Comment thread xmodule/x_module.py Outdated
@keithgg
keithgg requested a review from giovannicimolin July 7, 2022 07:58
@keithgg

keithgg commented Jul 7, 2022

Copy link
Copy Markdown
Contributor Author

@giovannicimolin @pkulkark I've made the requested changes. Please take a look.

Comment thread xmodule/x_module.py Outdated

@pkulkark pkulkark left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@keithgg Just the one nit about using dictionary object over list. Otherwise LGTM 👍

👍

  • I tested this: Verified this works as expected (on a different sandbox)
  • I read through the code
  • I checked for accessibility issues N/A
  • Includes documentation N/A

@pkulkark pkulkark assigned keithgg and unassigned giovannicimolin Jul 11, 2022
@pkulkark
pkulkark self-requested a review July 11, 2022 10:22
@pkulkark
pkulkark requested review from pkulkark and removed request for giovannicimolin July 11, 2022 12:09
@pkulkark

Copy link
Copy Markdown
Contributor

@giovannicimolin It looks like I can't merge this unless you add your approval too. Unless I don't have the right access to merge (I should have since I'm a core committer)? CC @natabene

@giovannicimolin

Copy link
Copy Markdown
Contributor

@pkulkark Approved based on your review.

This setting allows loading of Resource Templates from outside the
edx-platform codebase.

Operators will be able to add their own custom resource templates
without needing to fork the codebase.
@keithgg
keithgg force-pushed the keith/bb_5583_themed_html_templates branch from b54c79e to ed81774 Compare July 20, 2022 06:45
@pkulkark
pkulkark merged commit 145a30a into openedx:master Jul 20, 2022
@openedx-webhooks

Copy link
Copy Markdown

@keithgg 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@pkulkark
pkulkark deleted the keith/bb_5583_themed_html_templates branch July 20, 2022 07:12
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

1 similar comment
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

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

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

7 participants