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: include the correct SCSS file for lti_block #33155

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Sep 1, 2023

Testing

I confirmed this fix using Tutor and an export of the course from the original (private) bug report. Here's how I tested.

Setup:

  • Running instance of Studio & LMS
  • Course exists with "lti" added as an Advanced Module Type (in Advanced Settings) and a unit containing an "LTI" component (not an "LTI Consumer" component -- that is a newer implementation of the block, which didn't suffer from this bug).
  • LTI component should be pre-configured with an external resource.
  • Logged-in user can edit course

Bug reproduction (master)

  • Go to unit in Studio
  • Hit "Edit" on LTI component
  • Editor should show error message containing Error: Sass not found: /edx/app/edxapp/edx-platform/xmodule/assets/LTIBlockEditor.scss
  • Hit "View Live"
  • LTI contents should be squished into a very small frame in LMS -- about 300x400, by my guess.
  • Open network log, hard-refresh, and confirm that LTIBlockDisplay.css is NOT loaded.

Bug fix confirmation (this branch):

  • Go to unit in Studio
  • Hit "Edit" on LTI component
  • Confirm that editor displays and works.
  • Hit "View Live"
  • LTI contents should display in LMS in a frame of appropriate size.
  • Open network log, hard-refresh, and confirm that LTIBlockDisplay.css IS loaded.

Description

lti_block has Sass for its display, but not for its editor.

During the add_sass_to_fragment refactoring, I mixed this up: I added a non-existent scss file to the studio_view but didn't add the actual scss file to the student_view.

Course authors using the (deprecated) lti_block saw:

We're having trouble rendering your component
Students will not be able to access this component. Re-edit your component to fix the error.

Error: Sass not found: /edx/app/edxapp/edx-platform/xmodule/assets/LTIBlockEditor.scss

as a result of this bug.

Original PR: #32592

Private-ref: https://2u-internal.atlassian.net/browse/TNL-11029

lti_block has Sass for its display, but not for its editor.

During the `add_sass_to_fragment` refactoring, I mixed this up:
I added a non-existent scss file to the studio_view but didn't add
the actual scss file to the student_view.

Course authors using the (deprecated) lti_block saw:

    We're having trouble rendering your component
    Students will not be able to access this component. Re-edit your component to fix the error.

    Error: Sass not found: /edx/app/edxapp/edx-platform/xmodule/assets/LTIBlockEditor.scss

as a result of this bug.

Original PR: openedx#32592

Private-ref: https://2u-internal.atlassian.net/browse/TNL-11029
@kdmccormick kdmccormick merged commit 6704901 into openedx:master Sep 1, 2023
42 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/lti-sass-fix branch September 1, 2023 17:28
@edx-pipeline-bot
Copy link
Contributor

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

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

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.

3 participants