Harvard RCE tweaks

- not directly related to the RCE, but now preload the
3 lato-exended font variants likely to be used
  on the page. this keeps font loading from blocking
- the big change is to make it possible to limit the number
  of RCEs that will fully render on page load. This should
  address the loading issues for quizzes with many essay questions
  (the sample quiz from Harvard had 30 + other questions)
  For now, the necessary property is only being set on the
  take-quiz page

closes: MAT-355
flag=rce_enhancements,flag=rce_limit_init_render_on_page

test plan:
    account flag.
  - create a legacy quiz with > 5 essay questions. Feel
    free to have other questions in there too if you like
  - preview or take the quiz
  > expect all 6 RCEs to be created on the page
    - you could use React dev tools or
      document.querySelectorAll('.rce-wrapper').length === 6
  - turn the "RCE Limit number of RCEs initially rendered on th page"
    account feature flag on
  - take the quiz again
  > expect only the first 5 RCEs to be fully rendered
  - scroll down to the bring the 6th into view
  > expect it to be fully realized
  - while scrolled to the bottom, refresh the page
  > not that the 6th RCE is in view, expect it to be
    fully realized also

Change-Id: Idd76a56c4ea69e45a4f1cc28e3cd8561b40c2403
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/270075
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Weston Dransfield <wdransfield@instructure.com>
Reviewed-by: Jon Scheiding <jon.scheiding@instructure.com>
Product-Review: Ed Schiebel <eschiebel@instructure.com>
This commit is contained in:
Ed Schiebel 2021-07-27 11:13:08 -04:00
parent c651786492
commit 9930acbbfb
16 changed files with 309 additions and 74 deletions

View File

@ -237,7 +237,7 @@ class ApplicationController < ActionController::Base
].freeze
JS_ENV_ROOT_ACCOUNT_FEATURES = [
:responsive_awareness, :responsive_misc, :product_tours, :module_dnd, :files_dnd, :unpublished_courses,
:usage_rights_discussion_topics, :inline_math_everywhere, :granular_permissions_manage_users
:usage_rights_discussion_topics, :inline_math_everywhere, :granular_permissions_manage_users, :rce_limit_init_render_on_page
].freeze
JS_ENV_BRAND_ACCOUNT_FEATURES = [
:embedded_release_notes

View File

@ -304,6 +304,11 @@ module ApplicationHelper
File.join('/dist', 'brandable_css', base_dir, "#{bundle_path}-#{cache[:combinedChecksum]}.css")
end
def font_url_for(nominal_font_href)
cache = BrandableCSS.font_path_cache()
cache[nominal_font_href] || nominal_font_href
end
def brand_variable(variable_name)
BrandableCSS.brand_variable_value(variable_name, active_brand_config)
end

View File

@ -21,6 +21,9 @@
<meta charset="utf-8">
<link rel="preconnect" href="https://fonts.gstatic.com/" crossorigin>
<% if Setting.get('disable_lato_extended', false) == 'false' %>
<link rel="preload" href="<%=font_url_for("/fonts/lato/extended/Lato-Regular.woff2")%>" as="font" type="font/woff2" crossorigin>
<link rel="preload" href="<%=font_url_for("/fonts/lato/extended/Lato-Bold.woff2")%>" as="font" type="font/woff2" crossorigin>
<link rel="preload" href="<%=font_url_for("/fonts/lato/extended/Lato-Italic.woff2")%>" as="font" type="font/woff2" crossorigin>
<%= stylesheet_link_tag(css_url_for('lato_extended')) %>
<% else %>
<%= stylesheet_link_tag(css_url_for('lato')) %>

View File

@ -7,6 +7,7 @@ paths:
file_checksums: public/dist/brandable_css/brandable_css_file_checksums.json
output_dir: public/dist/brandable_css
browsers_yml: config/browsers.yml
rev_manifest: public/dist/rev-manifest.json
indices:
handlebars:

View File

@ -26,3 +26,11 @@ rce_better_file_previewing:
display_name: RCE Better File Previewing
description: Preview files linked in the RCE in an overlay rather than a new browser tab.
applies_to: SiteAdmin
rce_limit_init_render_on_page:
state: hidden
display_name: RCE Limit number of RCEs initially rendered on the page
description: |-
Cap the number of RCEs rendered on initial page load. After that
initialize each as it is about to enter the viewport.
applies_to: RootAccount
root_opt_in: true

View File

@ -329,6 +329,38 @@ module BrandableCSS
fingerprint
end
# build a cache of nominal paths to font files to the decorated version we need to request
# e.g. "/fonts/lato/extended/Lato-Bold.woff2": "/dist/fonts/lato/extended/Lato-Bold-cccb897485.woff2"
# only track .woff2 font files since those will be the only ones ever asked for
# (truth be told, could limit it to just lato extended)
# this function is more or less modeled after combined_checksums
def font_path_cache
return @decorated_font_paths if defined?(@decorated_font_paths) && defined?(ActionController) && ActionController::Base.perform_caching
file = APP_ROOT.join(CONFIG.dig('paths', 'rev_manifest'))
# in reality, if the rev-manifest.json file is missing you won't get this far, but let's be careful anyway
return(
@decorated_font_paths =
JSON.parse(file.read).each_with_object({}) do |(k, v), memo|
memo["/#{k}"] = "/dist/#{v}" if k =~ /^fonts.*woff2/
end.freeze
) if file.exist?
# the file does not exist in production, we have a problem
if defined?(Rails) && Rails.env.production?
raise "#{file.expand_path} does not exist. You need to run brandable_css before you can serve css."
end
# for dev/test there might be cases where you don't want it to raise an exception
# if you haven't run `brandable_css` and the manifest file doesn't exist yet.
# eg: you want to test a controller action and you don't care that it links
# to a css file that hasn't been created yet.
@decorated_font_paths = {
anyfont: 'Error: unknown css checksum. you need to run brandable_css'
}.freeze
end
def all_fingerprints_for(bundle_path)
variants.each_with_object({}) do |variant, object|
object[variant] = cache_for(bundle_path, variant)

View File

@ -336,6 +336,7 @@
"build": "yarn run build:css && yarn run build:js",
"build:watch": "concurrently --raw \"yarn build:css:watch\" \"yarn build:js:watch\"",
"build:css": "brandable_css",
"build:css:compressed": "SASS_STYLE=compressed brandable_css",
"build:css:watch": "brandable_css --watch",
"build:js": "yarn run webpack-development",
"build:js:watch": "yarn run webpack",

View File

@ -168,6 +168,12 @@ RCE.propTypes = {
// array of lti tools available to the user
// {id, favorite} are all that's required, ther fields are ignored
ltiTools: ltiToolsPropType,
// if the rce_limit_init_render_on_page flag is on, this
// is the maximum number of RCEs that will render on page load.
// Any more than this will be deferred until it is nearly
// scrolled into view.
// if isNaN or <=0, render them all
maxInitRenderedRCEs: number,
// name:value pairs of attributes to add to the textarea
// tinymce creates as the backing store of the RCE
mirroredAttrs: objectOf(string),
@ -201,6 +207,7 @@ RCE.defaultProps = {
instRecordDisabled: false,
language: 'en',
liveRegion: () => document.getElementById('flash_screenreader_holder'),
maxInitRenderedRCEs: -1,
mirroredAttrs: {},
readOnly: false,
use_rce_pretty_html_editor: true,

View File

@ -270,6 +270,8 @@ class RCEWrapper extends React.Component {
plugins: PropTypes.arrayOf(PropTypes.string),
instRecordDisabled: PropTypes.bool,
highContrastCSS: PropTypes.arrayOf(PropTypes.string),
maxInitRenderedRCEs: PropTypes.number,
// feature flag related props
use_rce_pretty_html_editor: PropTypes.bool,
use_rce_buttons_and_icons: PropTypes.bool,
use_rce_a11y_checker_notifications: PropTypes.bool
@ -280,7 +282,8 @@ class RCEWrapper extends React.Component {
languages: [{id: 'en', label: 'English'}],
autosave: {enabled: false},
highContrastCSS: [],
ltiTools: []
ltiTools: [],
maxInitRenderedRCEs: -1
}
static skinCssInjected = false
@ -300,6 +303,7 @@ class RCEWrapper extends React.Component {
this.indicator = false
this._elementRef = React.createRef()
this._editorPlaceholderRef = React.createRef()
this._prettyHtmlEditorRef = React.createRef()
this._showOnFocusButton = null
@ -310,6 +314,11 @@ class RCEWrapper extends React.Component {
ht = `${ht}px`
}
const currentRCECount = document.querySelectorAll('.rce-wrapper').length
const maxInitRenderedRCEs = Number.isNaN(props.maxInitRenderedRCEs)
? RCEWrapper.defaultProps.maxInitRenderedRCEs
: props.maxInitRenderedRCEs
this.state = {
path: [],
wordCount: 0,
@ -325,8 +334,13 @@ class RCEWrapper extends React.Component {
headerDisp: 'static',
isTinyFullscreen: false
},
a11yErrorsCount: 0
a11yErrorsCount: 0,
shouldShowEditor:
typeof IntersectionObserver === 'undefined' ||
maxInitRenderedRCEs <= 0 ||
currentRCECount < maxInitRenderedRCEs
}
this.pendingEventHandlers = []
// Get top 2 favorited LTI Tools
this.ltiToolFavorites =
@ -605,6 +619,15 @@ class RCEWrapper extends React.Component {
return contentInsertion.existingContentToLinkIsImg(editor)
}
// since we may defer rendering tinymce, queue up any tinymce event handlers
tinymceOn(tinymceEventName, handler) {
if (this.state.shouldShowEditor) {
this.mceInstance().on(tinymceEventName, handler)
} else {
this.pendingEventHandlers.push({name: tinymceEventName, handler})
}
}
mceInstance() {
if (this.editor) {
return this.editor
@ -1334,12 +1357,15 @@ class RCEWrapper extends React.Component {
}
componentWillUnmount() {
if (this.state.shouldShowEditor) {
window.clearTimeout(this.blurTimer)
if (!this._destroyCalled) {
this.destroy()
}
this._elementRef.current.removeEventListener('keydown', this.handleKey, true)
this.observer?.disconnect()
this.mutationObserver?.disconnect()
this.intersectionObserver?.disconnect()
}
}
wrapOptions(options = {}) {
@ -1555,6 +1581,46 @@ class RCEWrapper extends React.Component {
}
componentDidMount() {
if (this.state.shouldShowEditor) {
this.editorReallyDidMount()
} else {
this.intersectionObserver = new IntersectionObserver(
entries => {
const entry = entries[0]
if (entry.isIntersecting || entry.intersectionRatio > 0) {
this.setState({shouldShowEditor: true})
}
},
// initialize the RCE when it gets close to entering the viewport
{root: null, rootMargin: '200px 0px', threshold: 0.0}
)
this.intersectionObserver.observe(this._editorPlaceholderRef.current)
}
}
componentDidUpdate(prevProps, prevState) {
if (this.state.shouldShowEditor) {
if (!prevState.shouldShowEditor) {
this.editorReallyDidMount()
this.intersectionObserver?.disconnect()
} else {
this.registerTextareaChange()
if (prevState.editorView !== this.state.editorView) {
this.setEditorView(this.state.editorView)
this.focusCurrentView()
}
if (prevProps.readOnly !== this.props.readOnly) {
this.mceInstance().mode.set(this.props.readOnly ? 'readonly' : 'design')
}
}
}
}
editorReallyDidMount() {
const myTiny = this.mceInstance()
this.pendingEventHandlers.forEach(e => {
myTiny.on(e.name, e.handler)
})
this.registerTextareaChange()
this._elementRef.current.addEventListener('keydown', this.handleKey, true)
// give the textarea its initial size
@ -1574,29 +1640,18 @@ class RCEWrapper extends React.Component {
// my portal will be the last one in the doc because tinyumce appends them
const tinymce_floating_toolbar_portal = portals[portals.length - 1]
if (tinymce_floating_toolbar_portal) {
this.observer = new MutationObserver((mutationList, _observer) => {
this.mutationObserver = new MutationObserver((mutationList, _observer) => {
mutationList.forEach(mutation => {
if (mutation.type === 'childList' && mutation.addedNodes.length > 0) {
this.handleFocusEditor(new Event('focus', {target: mutation.target}))
}
})
})
this.observer.observe(tinymce_floating_toolbar_portal, {childList: true})
this.mutationObserver.observe(tinymce_floating_toolbar_portal, {childList: true})
}
bridge.renderEditor(this)
}
componentDidUpdate(prevProps, prevState) {
this.registerTextareaChange()
if (prevState.editorView !== this.state.editorView) {
this.setEditorView(this.state.editorView)
this.focusCurrentView()
}
if (prevProps.readOnly !== this.props.readOnly) {
this.mceInstance().mode.set(this.props.readOnly ? 'readonly' : 'design')
}
}
setEditorView(view) {
switch (view) {
case RAW_HTML_EDITOR_VIEW:
@ -1682,6 +1737,18 @@ class RCEWrapper extends React.Component {
render() {
const {trayProps, ...mceProps} = this.props
if (!this.state.shouldShowEditor) {
return (
<div
ref={this._editorPlaceholderRef}
style={{
width: `${this.props.editorOptions.width}px`,
height: `${this.props.editorOptions.height}px`,
border: '1px solid grey'
}}
/>
)
}
return (
<div
key={this.id}

View File

@ -71,6 +71,7 @@ export default function wrapInitCb(mirroredAttrs, editorOptions, MutationObserve
$(window).triggerHandler('resize')
if (!window.ENV?.use_rce_enhancements) {
// this is a hack so that when you drag an image from the sidebar to the editor that it doesn't
// try to embed the thumbnail but rather the full size version of the image.
// so basically, to document why and how this works: in wiki_sidebar.js we add the
@ -80,10 +81,15 @@ export default function wrapInitCb(mirroredAttrs, editorOptions, MutationObserve
$(ed.contentDocument).bind('DOMNodeInserted', e => {
const target = e.target
let mceSrc
if (target.nodeType === 1 && target.nodeName === 'IMG' && (mceSrc = $(target).data('url'))) {
if (
target.nodeType === 1 &&
target.nodeName === 'IMG' &&
(mceSrc = $(target).data('url'))
) {
$(target).attr('src', tinymce.activeEditor.documentBaseURI.toAbsolute(mceSrc))
}
})
}
// tiny sets a focusout event handler, which only IE supports
// (Chrome/Safari/Opera support DOMFocusOut, FF supports neither)

View File

@ -17,7 +17,6 @@
*/
import assert from 'assert'
import jsdomify from 'jsdomify'
import sinon from 'sinon'
import Bridge from '../../src/bridge'
import * as indicateModule from '../../src/common/indicate'
@ -32,6 +31,7 @@ import RCEWrapper, {
const textareaId = 'myUniqId'
let React, fakeTinyMCE, editorCommandSpy, sd, editor
let failedCount = 0
// ====================
// HELPERS
@ -88,11 +88,13 @@ function trayProps() {
// to provide the default props
function defaultProps() {
return {
textareaId,
highContrastCSS: [],
languages: [{id: 'en', label: 'English'}],
autosave: {enabled: false},
ltiTools: [],
editorOptions: {}
editorOptions: {},
liveRegion: () => document.getElementById('flash_screenreader_holder')
}
}
@ -102,16 +104,14 @@ describe('RCEWrapper', () => {
// ====================
beforeEach(() => {
jsdomify.create(`
<!DOCTYPE html><html dir="ltr"><head></head><body>
<div id="flash_screenreader_holder"/>
document.body.innerHTML = `
<div id="flash_screenreader_holder" role="alert"/>
<div id="app">
<textarea id="${textareaId}" />
</div>
</body></html>
`)
`
document.documentElement.dir = 'ltr'
// I don't know why this mocha tests suite uses jsdom, but it does.
// mock MutationObserver
if (!global.MutationObserver) {
global.MutationObserver = function MutationObserver(_props) {
@ -119,7 +119,6 @@ describe('RCEWrapper', () => {
}
}
// must create react after jsdom setup
requireReactDeps()
editorCommandSpy = sinon.spy()
editor = {
@ -157,7 +156,11 @@ describe('RCEWrapper', () => {
editor.content += contentToInsert
},
getContainer: () => {
return {}
return {
style: {
height: 300
}
}
},
setContent: sinon.spy(c => (editor.content = c)),
getContent: () => editor.content,
@ -171,7 +174,8 @@ describe('RCEWrapper', () => {
execCommand: editorCommandSpy,
serializer: {serialize: sinon.stub()},
ui: {registry: {addIcon: () => {}}},
isDirty: () => false
isDirty: () => false,
fire: () => {}
}
fakeTinyMCE = {
@ -192,8 +196,16 @@ describe('RCEWrapper', () => {
sinon.spy(editor, 'insertContent')
})
afterEach(() => {
jsdomify.destroy()
afterEach(function () {
if (this.currentTest.state === 'failed') {
++failedCount
}
document.body.innerHTML = ''
})
after(() => {
// I don't know why, but this this suite of tests stopped exiting
process.exit(failedCount ? 1 : 0)
})
// ====================
@ -355,7 +367,6 @@ describe('RCEWrapper', () => {
describe('insertImagePlaceholder', () => {
let globalImage
function mockImage(props) {
// jsdom doesn't support Image
// mock enough for RCEWrapper.insertImagePlaceholder
globalImage = global.Image
global.Image = function () {
@ -899,10 +910,6 @@ describe('RCEWrapper', () => {
})
describe('alert area', () => {
afterEach(() => {
jsdomify.destroy()
})
it('adds an alert and attaches an id when addAlert is called', () => {
const tree = createdMountedElement()
const rce = tree.getMountedInstance()
@ -1264,4 +1271,71 @@ describe('RCEWrapper', () => {
])
})
})
describe('limit the number or RCEs fully rendered on page load', () => {
let ReactDOM
before(() => {
ReactDOM = require('react-dom')
global.IntersectionObserver = function () {
return {
observe: () => {},
disconnect: () => {}
}
}
})
beforeEach(() => {
document.getElementById('app').innerHTML = `
<div class='rce-wrapper'>faux rendered rce</div>
<div class='rce-wrapper'>faux rendered rce</div>
<div id="here"/>
`
})
it('renders them all if no max is set', done => {
ReactDOM.render(
<RCEWrapper {...defaultProps()} tinymce={fakeTinyMCE} />,
document.getElementById('here'),
() => {
assert.strictEqual(document.querySelectorAll('.rce-wrapper').length, 3)
done()
}
)
})
it('renders them all if maxInitRenderedRCEs is <0', done => {
ReactDOM.render(
<RCEWrapper {...defaultProps()} tinymce={fakeTinyMCE} maxInitRenderedRCEs={-1} />,
document.getElementById('here'),
() => {
assert.strictEqual(document.querySelectorAll('.rce-wrapper').length, 3)
done()
}
)
})
it('limits them to maxInitRenderedRCEs value', done => {
ReactDOM.render(
<RCEWrapper {...defaultProps()} tinymce={fakeTinyMCE} maxInitRenderedRCEs={2} />,
document.getElementById('here'),
() => {
assert.strictEqual(document.querySelectorAll('.rce-wrapper').length, 2)
done()
}
)
})
it('copes with missing IntersectionObserver', done => {
delete global.IntersectionObserver
ReactDOM.render(
<RCEWrapper {...defaultProps()} tinymce={fakeTinyMCE} maxInitRenderedRCEs={2} />,
document.getElementById('here'),
() => {
assert.strictEqual(document.querySelectorAll('.rce-wrapper').length, 3)
done()
}
)
})
})
})

View File

@ -77,6 +77,9 @@ QUnit.module('loadOnTarget', {
callback()
}
}
},
tinymceOn(eventType, callback) {
callback()
}
}
this.rce = {renderIntoDiv: sinon.stub().callsArgWith(2, this.editor)}

View File

@ -148,4 +148,20 @@ describe BrandableCSS do
end
end
end
describe 'font_path_cache' do
it 'creates the cache' do
BrandableCSS.font_path_cache()
expect(BrandableCSS.instance_variable_get(:@decorated_font_paths)).not_to be_nil
end
it 'maps font paths' do
cache = BrandableCSS.font_path_cache
cache.each do |key, val|
expect(key).to start_with('/fonts')
expect(val).to start_with("/dist/fonts")
expect(val).to match(/-[a-z0-9]+\.woff2$/)
end
end
end
end

View File

@ -939,7 +939,8 @@ $(function() {
$(this).attr('id', 'question_input_' + quizSubmission.contentBoxCounter++)
RichContentEditor.loadNewEditor($(this), {
manageParent: true,
autosave: {enabled: false}
autosave: {enabled: false},
maxInitRenderedRCEs: 5
})
})
}, 2000)

View File

@ -99,6 +99,9 @@ const CanvasRce = forwardRef(function CanvasRce(props, rceRef) {
languages={languages}
liveRegion={() => document.getElementById('flash_screenreader_holder')}
ltiTools={window.INST?.editorButtons}
maxInitRenderedRCEs={
window.ENV?.FEATURES?.rce_limit_init_render_on_page ? props.maxInitRenderedRCEs : -1
}
mirroredAttrs={mirroredAttrs}
readOnly={readOnly}
textareaClassName={textareaClassName}
@ -129,6 +132,12 @@ CanvasRce.propTypes = {
editorOptions: object,
// height of the RCE. If a number, in px
height: oneOfType([number, string]),
// if the rce_limit_init_render_on_page flag is on, this
// is the maximum number of RCEs that will render on page load.
// Any more than this will be deferred until it is nearly
// scrolled into view.
// if isNaN or <=0, render them all
maxInitRenderedRCEs: number,
// name:value pairs of attributes to add to the textarea
// tinymce creates as the backing store of the RCE
mirroredAttrs: objectOf(string),
@ -148,6 +157,7 @@ CanvasRce.propTypes = {
CanvasRce.defaultProps = {
autosave: true,
editorOptions: {},
maxInitRenderedRCEs: -1,
mirroredAttrs: {},
readOnly: false,
textareaClassName: 'input-block-level',

View File

@ -39,9 +39,7 @@ const RCELoader = {
this.loadRCE(RCE => {
RCE.renderIntoDiv(renderingTarget, propsForRCE, remoteEditor => {
remoteEditor
.mceInstance()
.on('init', () => callback(textarea, polyfill.wrapEditor(remoteEditor)))
remoteEditor.tinymceOn('init', () => callback(textarea, polyfill.wrapEditor(remoteEditor)))
})
})
},
@ -211,6 +209,9 @@ const RCELoader = {
ltiTools: window.INST?.editorButtons,
autosave: tinyMCEInitOptions.autosave || autosave,
instRecordDisabled: ENV.RICH_CONTENT_INST_RECORD_TAB_DISABLED,
maxInitRenderedRCEs: !!window.ENV?.FEATURES?.rce_limit_init_render_on_page
? tinyMCEInitOptions.maxInitRenderedRCEs
: -1,
highContrastCSS: window.ENV?.url_for_high_contrast_tinymce_editor_css,
use_rce_pretty_html_editor: !!window.ENV?.FEATURES?.rce_pretty_html_editor,
use_rce_buttons_and_icons: !!window.ENV?.FEATURES?.rce_buttons_and_icons,