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: html tables rendering for subclasses of HTML5XBlock in studio preview #20

Merged
merged 3 commits into from
Oct 20, 2022

Conversation

Cup0fCoffee
Copy link
Member

@Cup0fCoffee Cup0fCoffee commented Oct 20, 2022

Description

This PR is a follow up fix to #18. Previous changes fixed rendering of the html tables in studio preview for HTML5XBlock. This PR changes the fix to work for subclasses of HTML5XBlock, like ExcludedHTML5XBlock and CompletableHTML5XBlock.

Screenshots

HTML used for screenshots:

<table style="border-collapse: collapse; width: 100%; height: 46px;" border="1">
<tbody>
<tr style="height: 18px;">
<td style="width: 15.4861%; height: 18px; background-color: #c2e0f4; border: 3px solid #000000;"><strong>Column 1</strong></td>
<td style="width: 21.4427%; height: 18px; background-color: #c2e0f4; border: 3px solid #000000;"><strong>Column 2</strong></td>
<td style="width: 26.8784%; background-color: #c2e0f4; height: 18px; border: 3px solid #000000;"><strong>Column 3</strong></td>
<td style="width: 35.1816%; height: 18px; background-color: #c2e0f4; border: 3px solid #000000;"><strong>Column 4</strong></td>
</tr>
<tr style="height: 18px;">
<td style="width: 15.4861%; height: 18px; background-color: #fbeeb8;">A2</td>
<td style="width: 21.4427%; height: 18px; background-color: #fbeeb8;">B2</td>
<td style="width: 26.8784%; height: 18px; background-color: #fbeeb8;">C2</td>
<td style="width: 35.1816%; height: 18px; background-color: #fbeeb8;">D2</td>
</tr>
<tr style="height: 10px;">
<td style="width: 15.4861%; height: 10px; background-color: #fbeeb8;">A3</td>
<td style="width: 21.4427%; height: 10px; background-color: #fbeeb8;">B3</td>
<td style="width: 26.8784%; height: 10px; background-color: #fbeeb8;">C3</td>
<td style="width: 35.1816%; height: 10px; background-color: #fbeeb8;">D3</td>
</tr>
<tr style="height: 0px;">
<td style="width: 15.4861%; height: 0px; background-color: #fbeeb8;">A4</td>
<td style="width: 21.4427%; height: 0px; background-color: #fbeeb8;">B4</td>
<td style="width: 26.8784%; height: 0px; background-color: #fbeeb8;">C4</td>
<td style="width: 35.1816%; height: 0px; background-color: #fbeeb8;">D4</td>
</tr>
</tbody>
</table>
Before

image

After

image

Testing instructions

  1. Setup devstack (for development Maple was used, but Nutmeg or master should also work)
  2. Clone xblock-html repo into <devstack-root>/src/ and change the branch
  3. Clone xblock-html-completable repo into <devstack-root>/src/
  4. Start lms, studio, and learning MFE:
     make lms-up studio-up frontend-app-learning-up
  5. Install xblock-html and xblock-html-completable into studio:
    make studio-shell studio-restart
    pip install -e /edx/src/xblock-html/ /edx/src/xblock-html-completable/ && exit
  6. Login into studio as staff (e.g. edx@example.com)
  7. Go to demo course advanced settings, and add html5, excluded_html5 and completable_html5 in Advanced Module List
  8. Go to demo course outline, and create a new section, subsection and unit
  9. In the new unit add html5 xblock - Advanced > Text
  10. In the new unit add excluded_html5 xblock - Advanced > Exclusion
  11. In the new unit add completable_html5 xblock - Advanced > Completable
  12. Create a new table using the visual editor in each XBlock
  13. Check that the fix works for all XBlocks, i.e. tables in all three XBlocks have borders

Private-ref: BB-6822

Previously a fix was added for how the html tables are rendered in the
studio preview. However, it only worked for HTML5XBlock xblock, and not
for it's subclasses.

This commit changes the fix to work for subclasses, like
ExcludedHTML5XBlock and CompletableHTML5XBlock.
Copy link
Member

@Agrendalath Agrendalath left a comment

Choose a reason for hiding this comment

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

👍

  • I tested this: Studio styles no longer rely on the XBlock type
  • I read through the code
  • I checked for accessibility issues: n/a
  • Includes documentation: n/a
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository

@Agrendalath Agrendalath merged commit 4b7828b into master Oct 20, 2022
@Agrendalath Agrendalath deleted the maxim/follow-up-fix-tables-rendering branch October 20, 2022 16:05
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