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

feat: update preview url to direct to mfe #35687

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

KristinAoki
Copy link
Member

@KristinAoki KristinAoki commented Oct 21, 2024

Description

This PR changes unit preview URL. Currently, unit previews render in the legacy learners page. As the learning MFE is developed and refiend, the UI between the legacy page and the MFE begins to diverge. This make it difficult for author to accurately determine what their content will look like for learners before it is published. With the PRs change, unit previews will no longer redirect to the legacy page, but the learning MFE. Additionally, this PR updates the content of the unit iframe depending on if it is in a preview or learner view. This change impact Authors. Some follow-up work may be needed to delete config for preview.host

Supporting information

JIRA Ticket: AU-1067 🔒

The Preview environment has drifted a bit away from the live view. The particular thing I spotted was that all the unit content is in an iframe, and preview doesn’t. There are some visual differences too, like the size of navigation elements, the placement of the bottom-of-page Previous/Next buttons, etc.

Things still look fairly accurate, but if you’re testing javascript, it can be a big difference.

Testing instructions

  1. Open a course in Studio
  2. Publish an existing unit
  3. Make a change to the unit so that it is showing a draft version in Studio
  4. Click "View live"
  5. Confirm that the published version of unit is visible
  6. Confirm that preview is not in the URL
  7. Go back to the Studio page
  8. Click "Preview"
  9. Confirm that the draft version of the unit is visible
  10. Confirm that "preview" is at the beginning of the URL

Deadline

None

Other information

This PR is dependent on frontend-app-learning PR #1501

Copy link
Contributor

@nsprenkle nsprenkle left a comment

Choose a reason for hiding this comment

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

Mostly looks good, need to remove the print statements before approval.

Otherwise, have tested locally and works 👍

cms/djangoapps/contentstore/utils.py Outdated Show resolved Hide resolved
cms/djangoapps/contentstore/views/component.py Outdated Show resolved Hide resolved
lms/djangoapps/courseware/tests/test_views.py Outdated Show resolved Hide resolved
lms/djangoapps/courseware/views/index.py Outdated Show resolved Hide resolved
lms/djangoapps/courseware/views/views.py Outdated Show resolved Hide resolved
openedx/features/course_experience/url_helpers.py Outdated Show resolved Hide resolved
Copy link
Contributor

@nsprenkle nsprenkle left a comment

Choose a reason for hiding this comment

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

Some unit tests appear to be breaking but otherwise code looks good :shipit:

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