Skip to content

Commit

Permalink
Merge pull request #18 from open-craft/maxim/fix-tables-rendering
Browse files Browse the repository at this point in the history
fix: rendering tables [BB-6729]
  • Loading branch information
Agrendalath authored Sep 30, 2022
2 parents 8e39d43 + ee50083 commit 18c6e79
Show file tree
Hide file tree
Showing 13 changed files with 200 additions and 50 deletions.
44 changes: 42 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
defaults: &DEFAULT
working_directory: /home/circleci/xblock-html
docker:
- image: circleci/python:3.8
- image: cimg/python:3.8


version: 2
Expand All @@ -11,7 +11,7 @@ jobs:
steps:
- checkout
- restore_cache:
key: deps1-{{ .Branch }}--{{ checksum "requirements/base.txt" }}-{{ checksum "requirements/test.txt" }}-{{ checksum "requirements/quality.txt" }}
key: deps2-{{ .Branch }}--{{ checksum "requirements/base.txt" }}-{{ checksum "requirements/test.txt" }}-{{ checksum "requirements/quality.txt" }}
- run:
name: Installing requirements
command: |
Expand Down Expand Up @@ -41,6 +41,22 @@ jobs:
name: Running tests
command: make unit-coverage

test_nutmeg:
<<: *DEFAULT
steps:
- checkout
- attach_workspace:
at: /tmp/workspace
- run:
name: Activate virtualenv
command: echo 'source /tmp/workspace/env/bin/activate' >> $BASH_ENV
- run:
name: Install bleach 5
command: pip install bleach[css]==5.0.0
- run:
name: Running tests
command: make unit-coverage

quality:
<<: *DEFAULT
steps:
Expand All @@ -54,6 +70,22 @@ jobs:
name: Test Quality
command: make quality

quality_nutmeg:
<<: *DEFAULT
steps:
- checkout
- attach_workspace:
at: /tmp/workspace
- run:
name: Activate virtualenv
command: echo 'source /tmp/workspace/env/bin/activate' >> $BASH_ENV
- run:
name: Install bleach 5
command: pip install bleach[css]==5.0.0
- run:
name: Test Quality
command: make quality

workflows:
version: 2
build-test:
Expand All @@ -65,3 +97,11 @@ workflows:
- quality:
requires:
- build
- test_nutmeg:
requires:
- test
- quality
- quality_nutmeg:
requires:
- test
- quality
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ test_requirements: base_requirements ## Install requirements needed by test envi
pip install -q -r requirements/test.txt --exists-action w

requirements: base_requirements test_requirements ## Installs all requirements needed by developmenent and test environments
pip install -e .
@echo "Finished installing requirements."

quality: ## Run quality tests and checks
Expand All @@ -48,7 +47,7 @@ unit-coverage: clean ## Run coverage and unit tests
coverage run ./manage.py test
coverage html
coverage xml
diff-cover coverage.xml --html-report diff-cover.html
diff-cover coverage.xml --compare-branch origin/master --html-report diff-cover.html

unit: clean ## Run unit tests
mkdir var/
Expand Down
23 changes: 16 additions & 7 deletions html_xblock/bleaching.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
# NOTE:
# The bleach library changes the way CSS Styles are cleaned in
# version 5.0.0. Since the edx-platform uses version 4.1.0 in
# Maple and Nutmeg, this import is handled within a try block.
# This try block CAN BE REMOVED after Olive
# Maple, this import is handled within a try block.
# This try block CAN BE REMOVED for Nutmeg
CSSSanitizer = None


Expand Down Expand Up @@ -42,6 +42,7 @@ def get_cleaner(self):
considering safe in the platform.
"""
if CSSSanitizer:
# pylint: disable-next=unexpected-keyword-arg
cleaner = bleach.Cleaner(
tags=self._get_allowed_tags(),
attributes=self._get_allowed_attributes(),
Expand All @@ -51,9 +52,8 @@ def get_cleaner(self):
)
else:
# NOTE: This is maintaining backward compatibility with bleach 4.1.0
# used in Maple and Nutmeg release of edx-platform. This can be removed
# for Olive release which uses bleach 5.0.0

# used in Maple release of edx-platform. This can be removed
# for Nutmeg release which uses bleach 5.0.0
# pylint: disable-next=unexpected-keyword-arg
cleaner = bleach.Cleaner(
tags=self._get_allowed_tags(),
Expand Down Expand Up @@ -91,8 +91,11 @@ def _get_allowed_tags(self):
'ul',
'p',
'pre',
'caption',
'table',
'thead',
'tbody',
'tfoot',
'th',
'tr',
'td'
Expand All @@ -118,7 +121,9 @@ def _get_allowed_attributes(self):
'pre': ['class'],
'span': ['style'],
'ul': [],
'table': ['class'],
'table': ['class', 'style', 'border', 'cellspacing', 'cellpadding'],
'tr': ['style'],
'td': ['style', 'scope'],
}

if not self.strict:
Expand All @@ -136,7 +141,11 @@ def _get_allowed_styles(self):
:return: Allowed styles depending on the bleaching mode
"""
styles = ['font-family', 'text-align', 'color', 'text-decoration', 'padding-left', 'padding-right']
styles = [
'background-color', 'border', 'border-collapse', 'border-color', 'border-style', 'color', 'font-family',
'height', 'margin-left', 'margin-right', 'padding-left', 'padding-right', 'text-align', 'text-decoration',
'vertical-align', 'width',
]

if not self.strict:
styles += ['list-style-type', 'font-size', 'border-width', 'margin']
Expand Down
19 changes: 11 additions & 8 deletions html_xblock/html.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ def student_view(self, context=None): # pylint: disable=unused-argument
frag.add_css(self.resource_string('public/plugins/codesample/css/prism.css'))
frag.add_javascript(self.resource_string('public/plugins/codesample/js/prism.js'))

if getattr(self.runtime, 'is_author_mode', False):
frag.add_css(self.resource_string('static/css/html_preview.css'))

return frag

def studio_view(self, context=None): # pylint: disable=unused-argument
Expand All @@ -97,8 +100,8 @@ def studio_view(self, context=None): # pylint: disable=unused-argument

frag.content = xblock_loader.render_django_template('static/html/studio.html', context)

self.add_stylesheets(frag)
self.add_scripts(frag)
self.add_edit_stylesheets(frag)
self.add_edit_scripts(frag)

js_data = {
'editor': self.editor,
Expand Down Expand Up @@ -152,24 +155,24 @@ def workbench_scenarios():
"""),
]

def add_stylesheets(self, frag):
def add_edit_stylesheets(self, frag):
"""
A helper method to add all necessary styles to the fragment.
A helper method to add all styles to the fragment necesesary for edit.
:param frag: The fragment that will hold the scripts.
"""
frag.add_css(self.resource_string('static/css/html.css'))
frag.add_css(self.resource_string('static/css/html_edit.css'))

if self.editor == 'raw':
frag.add_css_url(settings.STATIC_URL + 'js/vendor/CodeMirror/codemirror.css')

def add_scripts(self, frag):
def add_edit_scripts(self, frag):
"""
A helper method to add all necessary scripts to the fragment.
A helper method to add all scripts to the fragment necessary for edit.
:param frag: The fragment that will hold the scripts.
"""
frag.add_javascript_url(settings.STATIC_URL + 'js/vendor/tinymce/js/tinymce/tinymce.full.min.js')
frag.add_javascript_url(settings.STATIC_URL + 'js/vendor/tinymce/js/tinymce/themes/silver/theme.min.js')
frag.add_javascript(self.resource_string('static/js/html.js'))
frag.add_javascript(self.resource_string('static/js/html_edit.js'))
frag.add_javascript(loader.load_unicode('public/studio_edit.js'))

if self.editor == 'raw':
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
/* .modal-type-html5 [class*="view-"] .modal-lg.modal-editor .modal-content {
height: 500px;
} */

.html-editor,
.wrapper-comp-settings.is-active.editor-with-buttons {
height: 320px;
Expand Down
77 changes: 77 additions & 0 deletions html_xblock/static/css/html_preview.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/* This css file contains styles that should be used for the XBlock preview in
* studio.
*
* It contains the fix for rendering tables in studio preview, which is
* caused by global studio styles. These global styles set `border: 0;` for
* tables and their elements, which removes the borders from these elements. */

/* fall back to default browser styles for all rows; the inline styles (like
* border style and color) would overwrite it */
.studio-xblock-wrapper [data-block-type="html5"] table > * > tr {
border: unset;
}

/* fall back to default browser styles, if border attribute is not set; the
* inline styles (like border style and color) would overwrite it */
.studio-xblock-wrapper [data-block-type="html5"] table:not([border]) {
border: unset;
}

/* default styles for any table that had border attribute set; the inline
* styles (like border style and color) would overwrite it */
.studio-xblock-wrapper [data-block-type="html5"] table[border] {
border: 1px solid black;
}

/* change the border width, depending on the explicit value of the border
* attribute */
.studio-xblock-wrapper [data-block-type="html5"] table[border="1"] {
border-width: 1px;
}

.studio-xblock-wrapper [data-block-type="html5"] table[border="2"] {
border-width: 2px;
}

.studio-xblock-wrapper [data-block-type="html5"] table[border="3"] {
border-width: 3px;
}

.studio-xblock-wrapper [data-block-type="html5"] table[border="4"] {
border-width: 4px;
}

.studio-xblock-wrapper [data-block-type="html5"] table[border="5"] {
border-width: 5px;
}

.studio-xblock-wrapper [data-block-type="html5"] table[border="6"] {
border-width: 6px;
}

.studio-xblock-wrapper [data-block-type="html5"] table[border="7"] {
border-width: 7px;
}

.studio-xblock-wrapper [data-block-type="html5"] table[border="8"] {
border-width: 8px;
}

.studio-xblock-wrapper [data-block-type="html5"] table[border="9"] {
border-width: 9px;
}

.studio-xblock-wrapper [data-block-type="html5"] table[border="10"] {
border-width: 10px;
}
/* etc until the value we think is reasonable */

.studio-xblock-wrapper [data-block-type="html5"] table[border] > * > tr > td,
.studio-xblock-wrapper [data-block-type="html5"] table[border] > * > tr > th,
.studio-xblock-wrapper [data-block-type="html5"] table[border] > * > td,
.studio-xblock-wrapper [data-block-type="html5"] table[border] > * > th,
.studio-xblock-wrapper [data-block-type="html5"] table[border] > td,
.studio-xblock-wrapper [data-block-type="html5"] table[border] > th {
border-width: thin;
border-style: inset;
}
File renamed without changes.
3 changes: 2 additions & 1 deletion pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@ min-similarity-lines=8
ignore=D101,D400,D401,D200,D203,D205,D212,D215,D404,D405,D406,D407,D408,D409,D410,D411,D412,D413,D414

[pycodestyle]
exclude = .git,migrations
max-line-length=120

# TODO: Use `edx_lint write pylintrc` to generate rules.
12 changes: 5 additions & 7 deletions requirements/base.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# Requirements for app run

git+https://github.com/edx/[email protected]#egg=xblock-utils==2.1.1
django-statici18n==1.9.0
edx-i18n-tools==0.5.3
Mako==1.1.3
bleach # version unpinned to make it compatible with Maple/Nutmeg (4.1.0)
# bleach[css]==5.0.0 # Use this for Olive
django~=2.2
xblock-utils==2.2.0
edx-i18n-tools==0.9.1
bleach==4.1.0 # version pinned for Maple
# bleach[css]==5.0.0 # Use this for Nutmeg
django==3.2.13
12 changes: 5 additions & 7 deletions requirements/quality.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# Requirements for code quality checks

isort==4.3.21
astroid==2.3.3
pycodestyle==2.6.0
pydocstyle==5.0.2
pylint==2.4.2
caniusepython3==7.2.0
edx-lint==1.4.1
isort==5.10.1
pycodestyle==2.9.1
pydocstyle==6.1.1
pylint==2.15.3
edx-lint==5.3.0
15 changes: 4 additions & 11 deletions requirements/test.txt
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
# Requirements for test runs

codecov==2.0.15
diff-cover==2.6.1
django-nose==1.4.4
lazy==1.4
pytest-cov==2.8.1
tox==3.15.0
tox-battery==0.6.0
mock==3.0.5

# Github requirements
git+https://github.com/edx/[email protected]#egg=django-pyfs==2.1
codecov==2.1.12
diff-cover==7.0.1
pytest-cov==4.0.0
mock==4.0.3
xblock-sdk
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def package_data(pkg, roots):

setup(
name='html-xblock',
version='1.2.0',
version='1.2.1',
description='HTML XBlock will help creating and using a secure and easy-to-use HTML blocks',
license='AGPL v3',
packages=[
Expand Down
Loading

0 comments on commit 18c6e79

Please sign in to comment.