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: modifications to remove wanings updated enrollment service #6

Merged
merged 4 commits into from
Mar 25, 2024

Conversation

ziafazal
Copy link
Contributor

fine tuning code and readme
Improved enrollment service API
Stopping inline iframe usage for now

Merge checklist:
Check off if complete or not applicable:

  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Fixup commits are squashed away
  • Unit tests added/updated
  • Manual testing instructions provided
  • Noted any: Concerns, dependencies, migration issues, deadlines, tickets

fine tuning code and readme
Improved enrollment service API
Stopping inline iframe usage for now
@ziafazal ziafazal requested review from hinakhadim and Talha-Rizwan and removed request for hinakhadim February 15, 2024 11:26
function updateIframeSrc(href, liText) {
if (liText.includes('AnyWindow')) {
function updateIframeSrc(href, launchMethod) {
// TODO: Uncomment the code below when we have figured out a workaround of limitations imposed by
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no commented code here.

{% if index_page_url %}
<h5 class="xblock-title mb-2"> {% trans "Available Assignables:" %}</h5>
<!-- <h5 class="xblock-title mb-2"> {% trans "Available Assignable Units:" %}</h5> -->
<div class="{% if cmi5_xblock.au_has_any_window %}cmi5-nav cmi5-column{% endif %}">
Copy link
Contributor

Choose a reason for hiding this comment

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

au_has_any_window is not being assigned any value except for default until this comment resolves.

Copy link
Contributor

@Talha-Rizwan Talha-Rizwan left a comment

Choose a reason for hiding this comment

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

The CMI5 xblock has an installation issue in tutor quince as it doesn't automatically install by tutor mounts add <path/to/xblock>. We can maybe add that in the README troubleshooting section.

Otherwise LGTM. 👍

@ziafazal ziafazal merged commit 994ca6f into master Mar 25, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants