Commit d1485d0b authored by Robert Knight's avatar Robert Knight

Convert `Excerpt` component to Preact

Convert the `Excerpt` component to Preact. Rather than convert the
existing implementation verbatim, this is a completely new implementation which
should be easier to understand and use. Instead of requiring callers to
provide an input property which represents the displayed data, which triggers a
re-measurement if it changes, the new implementation observes the DOM
directly for size changes.

This component renders caller-provided content (ie. it accepts a `children`
prop), which is not supported by the Preact <-> Angular bridge. Therefore it was
also necessary to create components (`AnnotationBody`, `AnnotationQuote`) that
encapsulate uses of `Excerpt` inside the `<annotation>` component.

 - Add `observe-element-size` utility module to watch for changes in
   the size of a DOM node using APIs available in the current browser

 - Add new `Excerpt` implementation and remove the old one

 - Remove `excerpt-overflow-monitor` utility that is not used by the new
   implementation

 - Add `AnnotationBody` component to render an annotation's markup body
   inside a (new) excerpt and convert the Angular template for
   `<annotation>` (annotation.html) to use it.

 - Add `AnnotationQuote` component to render an annotation's quote
   inside an excerpt and convert `annotation.html` to use it
parent d9abd85d
'use strict';
const { createElement } = require('preact');
const propTypes = require('prop-types');
const Excerpt = require('./excerpt');
const MarkdownEditor = require('./markdown-editor');
const MarkdownView = require('./markdown-view');
/**
* Display the rendered content of an annotation.
*/
function AnnotationBody({
collapse,
isEditing,
isHiddenByModerator,
hasContent,
onCollapsibleChanged,
onEditText,
onToggleCollapsed,
text,
}) {
return (
<section className="annotation-body">
{!isEditing && (
<Excerpt
collapse={collapse}
collapsedHeight={400}
inlineControls={false}
onCollapsibleChanged={onCollapsibleChanged}
onToggleCollapsed={collapsed => onToggleCollapsed({ collapsed })}
overflowHystersis={20}
>
<MarkdownView
markdown={text}
textClass={{
'annotation-body is-hidden': isHiddenByModerator,
'has-content': hasContent,
}}
/>
</Excerpt>
)}
{isEditing && <MarkdownEditor text={text} onEditText={onEditText} />}
</section>
);
}
AnnotationBody.propTypes = {
collapse: propTypes.bool,
hasContent: propTypes.bool,
isEditing: propTypes.bool,
isHiddenByModerator: propTypes.bool,
onCollapsibleChanged: propTypes.func,
onEditText: propTypes.func,
onToggleCollapsed: propTypes.func,
text: propTypes.string,
};
module.exports = AnnotationBody;
'use strict';
const classnames = require('classnames');
const { createElement } = require('preact');
const propTypes = require('prop-types');
const { withServices } = require('../util/service-context');
const { applyTheme } = require('../util/theme');
const Excerpt = require('./excerpt');
/**
* Display the selected text from the document associated with an annotation.
*/
function AnnotationQuote({ isOrphan, quote, settings = {} }) {
return (
<section
className={classnames('annotation-quote-list', isOrphan && 'is-orphan')}
>
<Excerpt
collapsedHeight={35}
inlineControls={true}
overflowHystersis={20}
>
<blockquote
className="annotation-quote"
style={applyTheme(['selectionFontFamily'], settings)}
>
{quote}
</blockquote>
</Excerpt>
</section>
);
}
AnnotationQuote.propTypes = {
isOrphan: propTypes.bool,
quote: propTypes.string,
// Used for theming.
settings: propTypes.object,
};
AnnotationQuote.injectedProps = ['settings'];
module.exports = withServices(AnnotationQuote);
...@@ -562,10 +562,6 @@ function AnnotationController( ...@@ -562,10 +562,6 @@ function AnnotationController(
return; return;
} }
self.canCollapseBody = canCollapse; self.canCollapseBody = canCollapse;
// This event handler is called from outside the digest cycle, so
// explicitly trigger a digest.
$scope.$digest();
}; };
this.setText = function(text) { this.setText = function(text) {
......
This diff is collapsed.
'use strict';
const { createElement } = require('preact');
const { mount } = require('enzyme');
const AnnotationBody = require('../annotation-body');
const mockImportedComponents = require('./mock-imported-components');
describe('AnnotationBody', () => {
function createBody(props = {}) {
return mount(<AnnotationBody text="test comment" {...props} />);
}
beforeEach(() => {
AnnotationBody.$imports.$mock(mockImportedComponents());
});
afterEach(() => {
AnnotationBody.$imports.$restore();
});
it('displays the comment if `isEditing` is false', () => {
const wrapper = createBody({ isEditing: false });
assert.isFalse(wrapper.exists('MarkdownEditor'));
assert.isTrue(wrapper.exists('MarkdownView'));
});
it('displays an editor if `isEditing` is true', () => {
const wrapper = createBody({ isEditing: true });
assert.isTrue(wrapper.exists('MarkdownEditor'));
assert.isFalse(wrapper.exists('MarkdownView'));
});
});
'use strict';
const { createElement } = require('preact');
const { mount } = require('enzyme');
const AnnotationQuote = require('../annotation-quote');
const mockImportedComponents = require('./mock-imported-components');
describe('AnnotationQuote', () => {
function createQuote(props) {
return mount(
<AnnotationQuote quote="test quote" settings={{}} {...props} />
);
}
beforeEach(() => {
AnnotationQuote.$imports.$mock(mockImportedComponents());
});
afterEach(() => {
AnnotationQuote.$imports.$restore();
});
it('renders the quote', () => {
const wrapper = createQuote();
const quote = wrapper.find('blockquote');
assert.equal(quote.text(), 'test quote');
});
});
...@@ -156,16 +156,22 @@ describe('annotation', function() { ...@@ -156,16 +156,22 @@ describe('annotation', function() {
onClick: '&', onClick: '&',
}, },
}) })
.component('markdownEditor', { .component('annotationBody', {
bindings: { bindings: {
text: '<', collapse: '<',
hasContent: '<',
isEditing: '<',
isHiddenByModerator: '<',
onCollapsibleChanged: '&',
onEditText: '&', onEditText: '&',
onToggleCollapsed: '&',
text: '<',
}, },
}) })
.component('markdownView', { .component('annotationQuote', {
bindings: { bindings: {
markdown: '<', isOrphan: '<',
textClass: '<', quote: '<',
}, },
}); });
}); });
...@@ -1256,18 +1262,6 @@ describe('annotation', function() { ...@@ -1256,18 +1262,6 @@ describe('annotation', function() {
}); });
}); });
it('renders quotes as plain text', function() {
const ann = fixtures.defaultAnnotation();
ann.target[0].selector = [
{
type: 'TextQuoteSelector',
exact: '<<-&->>',
},
];
const el = createDirective(ann).element;
assert.equal(el[0].querySelector('blockquote').textContent, '<<-&->>');
});
[ [
{ {
context: 'for moderators', context: 'for moderators',
...@@ -1275,10 +1269,8 @@ describe('annotation', function() { ...@@ -1275,10 +1269,8 @@ describe('annotation', function() {
// Content still present. // Content still present.
text: 'Some offensive content', text: 'Some offensive content',
}), }),
textClass: { isHiddenByModerator: true,
'annotation-body is-hidden': true, hasContent: true,
'has-content': true,
},
}, },
{ {
context: 'for non-moderators', context: 'for non-moderators',
...@@ -1287,18 +1279,17 @@ describe('annotation', function() { ...@@ -1287,18 +1279,17 @@ describe('annotation', function() {
tags: [], tags: [],
text: '', text: '',
}), }),
textClass: { isHiddenByModerator: true,
'annotation-body is-hidden': true, hasContent: false,
'has-content': false,
}, },
}, ].forEach(({ ann, context, isHiddenByModerator, hasContent }) => {
].forEach(testCase => { it(`passes moderation status to annotation body (${context})`, () => {
it(`renders hidden annotations with a custom text class (${testCase.context})`, () => { const el = createDirective(ann).element;
const el = createDirective(testCase.ann).element;
assert.match( assert.match(
el.find('markdown-view').controller('markdownView'), el.find('annotation-body').controller('annotationBody'),
sinon.match({ sinon.match({
textClass: testCase.textClass, isHiddenByModerator,
hasContent,
}) })
); );
}); });
......
This diff is collapsed.
...@@ -130,6 +130,10 @@ function startAngularApp(config) { ...@@ -130,6 +130,10 @@ function startAngularApp(config) {
// UI components // UI components
.component('annotation', require('./components/annotation')) .component('annotation', require('./components/annotation'))
.component(
'annotationBody',
wrapReactComponent(require('./components/annotation-body'))
)
.component( .component(
'annotationHeader', 'annotationHeader',
wrapReactComponent(require('./components/annotation-header')) wrapReactComponent(require('./components/annotation-header'))
...@@ -142,6 +146,10 @@ function startAngularApp(config) { ...@@ -142,6 +146,10 @@ function startAngularApp(config) {
'annotationPublishControl', 'annotationPublishControl',
wrapReactComponent(require('./components/annotation-publish-control')) wrapReactComponent(require('./components/annotation-publish-control'))
) )
.component(
'annotationQuote',
wrapReactComponent(require('./components/annotation-quote'))
)
.component( .component(
'annotationShareDialog', 'annotationShareDialog',
require('./components/annotation-share-dialog') require('./components/annotation-share-dialog')
...@@ -151,7 +159,6 @@ function startAngularApp(config) { ...@@ -151,7 +159,6 @@ function startAngularApp(config) {
'annotationViewerContent', 'annotationViewerContent',
require('./components/annotation-viewer-content') require('./components/annotation-viewer-content')
) )
.component('excerpt', require('./components/excerpt'))
.component( .component(
'helpPanel', 'helpPanel',
wrapReactComponent(require('./components/help-panel')) wrapReactComponent(require('./components/help-panel'))
...@@ -160,14 +167,6 @@ function startAngularApp(config) { ...@@ -160,14 +167,6 @@ function startAngularApp(config) {
'loggedOutMessage', 'loggedOutMessage',
wrapReactComponent(require('./components/logged-out-message')) wrapReactComponent(require('./components/logged-out-message'))
) )
.component(
'markdownEditor',
wrapReactComponent(require('./components/markdown-editor'))
)
.component(
'markdownView',
wrapReactComponent(require('./components/markdown-view'))
)
.component( .component(
'moderationBanner', 'moderationBanner',
wrapReactComponent(require('./components/moderation-banner')) wrapReactComponent(require('./components/moderation-banner'))
...@@ -232,7 +231,6 @@ function startAngularApp(config) { ...@@ -232,7 +231,6 @@ function startAngularApp(config) {
// Utilities // Utilities
.value('Discovery', require('../shared/discovery')) .value('Discovery', require('../shared/discovery'))
.value('ExcerptOverflowMonitor', require('./util/excerpt-overflow-monitor'))
.value('OAuthClient', require('./util/oauth-client')) .value('OAuthClient', require('./util/oauth-client'))
.value('VirtualThreadList', require('./virtual-thread-list')) .value('VirtualThreadList', require('./virtual-thread-list'))
.value('isSidebar', isSidebar) .value('isSidebar', isSidebar)
......
...@@ -13,45 +13,22 @@ ...@@ -13,45 +13,22 @@
show-document-info="vm.showDocumentInfo"> show-document-info="vm.showDocumentInfo">
</annotation-header> </annotation-header>
<!-- Excerpts --> <annotation-quote
<section class="annotation-quote-list" quote="vm.quote()"
ng-class="{'is-orphan' : vm.isOrphan()}" is-orphan="vm.isOrphan()"
ng-if="vm.quote()"> ng-if="vm.quote()">
<excerpt collapsed-height="35" </annotation-quote>
inline-controls="true"
overflow-hysteresis="20"
content-data="selector.exact">
<blockquote class="annotation-quote"
h-branding="selectionFontFamily"
ng-bind="vm.quote()"></blockquote>
</excerpt>
</section>
<!-- / Excerpts --> <annotation-body
<!-- Body -->
<section name="text" class="annotation-body">
<excerpt
inline-controls="false"
on-collapsible-changed="vm.setBodyCollapsible(collapsible)"
collapse="vm.collapseBody" collapse="vm.collapseBody"
collapsed-height="400" has-content="vm.hasContent()"
overflow-hysteresis="20" is-editing="vm.editing()"
content-data="vm.state().text" is-hidden-by-moderator="vm.isHiddenByModerator()"
ng-if="!vm.editing()"> on-collapsible-changed="vm.setBodyCollapsible(collapsible)"
<markdown-view
markdown="vm.state().text"
text-class="{'annotation-body is-hidden':vm.isHiddenByModerator(),
'has-content':vm.hasContent()}">
</markdown-view>
</excerpt>
<markdown-editor
text="vm.state().text"
on-edit-text="vm.setText(text)" on-edit-text="vm.setText(text)"
ng-if="vm.editing()"> on-toggle-collapsed="vm.collapseBody = collapsed"
</markdown-editor> text="vm.state().text">
</section> </annotation-body>
<!-- / Body -->
<!-- Tags --> <!-- Tags -->
<div class="annotation-body form-field" ng-if="vm.editing()"> <div class="annotation-body form-field" ng-if="vm.editing()">
......
<div class="excerpt__container">
<div class="excerpt" ng-style="vm.contentStyle()">
<div ng-transclude></div>
<div ng-click="vm.expand()"
ng-class="vm.bottomShadowStyles()"
title="Show the full excerpt"></div>
<div class="excerpt__inline-controls"
ng-show="vm.showInlineControls()">
<span class="excerpt__toggle-link" ng-show="vm.isExpandable()">
<a ng-click="vm.toggle($event)"
title="Show the full excerpt"
h-branding="accentColor, selectionFontFamily">More</a>
</span>
<span class="excerpt__toggle-link" ng-show="vm.isCollapsible()">
<a ng-click="vm.toggle($event)"
title="Show the first few lines only"
h-branding="accentColor, selectionFontFamily">Less</a>
</span>
</div>
</div>
</div>
'use strict';
function toPx(val) {
return val.toString() + 'px';
}
/**
* Interface used by ExcerptOverflowMonitor to retrieve the state of the
* <excerpt> and report when the state changes.
*
* interface Excerpt {
* getState(): State;
* contentHeight(): number | undefined;
* onOverflowChanged(): void;
* }
*/
/**
* A helper for the <excerpt> component which handles determinination of the
* overflow state and content styling given the current state of the component
* and the height of its contents.
*
* When the state of the excerpt or its content changes, the component should
* call check() to schedule an async update of the overflow state.
*
* @param {Excerpt} excerpt - Interface used to query the current state of the
* excerpt and notify it when the overflow state changes.
* @param {(callback) => number} requestAnimationFrame -
* Function called to schedule an async recalculation of the overflow
* state.
*/
function ExcerptOverflowMonitor(excerpt, requestAnimationFrame) {
let pendingUpdate = false;
// Last-calculated overflow state
let prevOverflowing;
function update() {
const state = excerpt.getState();
if (!pendingUpdate) {
return;
}
pendingUpdate = false;
const hysteresisPx = state.overflowHysteresis || 0;
const overflowing =
excerpt.contentHeight() > state.collapsedHeight + hysteresisPx;
if (overflowing === prevOverflowing) {
return;
}
prevOverflowing = overflowing;
excerpt.onOverflowChanged(overflowing);
}
/**
* Schedule a deferred check of whether the content is collapsed.
*/
function check() {
if (pendingUpdate) {
return;
}
pendingUpdate = true;
requestAnimationFrame(update);
}
/**
* Returns an object mapping CSS properties to values that should be applied
* to an excerpt's content element in order to truncate it based on the
* current overflow state.
*/
function contentStyle() {
const state = excerpt.getState();
let maxHeight = '';
if (prevOverflowing) {
if (state.collapse) {
maxHeight = toPx(state.collapsedHeight);
} else if (state.animate) {
// Animating the height change requires that the final
// height be specified exactly, rather than relying on
// auto height
maxHeight = toPx(excerpt.contentHeight());
}
} else if (typeof prevOverflowing === 'undefined' && state.collapse) {
// If the excerpt is collapsed but the overflowing state has not yet
// been computed then the exact max height is unknown, but it will be
// in the range [state.collapsedHeight, state.collapsedHeight +
// state.overflowHysteresis]
//
// Here we guess that the final content height is most likely to be
// either less than `collapsedHeight` or more than `collapsedHeight` +
// `overflowHysteresis`, in which case it will be truncated to
// `collapsedHeight`.
maxHeight = toPx(state.collapsedHeight);
}
return {
'max-height': maxHeight,
};
}
this.contentStyle = contentStyle;
this.check = check;
}
module.exports = ExcerptOverflowMonitor;
'use strict';
const ExcerptOverflowMonitor = require('../excerpt-overflow-monitor');
describe('ExcerptOverflowMonitor', function() {
let contentHeight;
let ctrl;
let monitor;
let state;
beforeEach(function() {
contentHeight = 0;
state = {
animate: true,
collapsedHeight: 100,
collapse: true,
overflowHysteresis: 20,
};
ctrl = {
getState: function() {
return state;
},
contentHeight: function() {
return contentHeight;
},
onOverflowChanged: sinon.stub(),
};
monitor = new ExcerptOverflowMonitor(ctrl, function(callback) {
callback();
});
});
describe('overflow state', function() {
it('overflows if height > collaped height + hysteresis', function() {
contentHeight = 200;
monitor.check();
assert.calledWith(ctrl.onOverflowChanged, true);
});
it('does not overflow if height < collapsed height', function() {
contentHeight = 80;
monitor.check();
assert.calledWith(ctrl.onOverflowChanged, false);
});
it('does not overflow if height is in [collapsed height, collapsed height + hysteresis]', function() {
contentHeight = 110;
monitor.check();
assert.calledWith(ctrl.onOverflowChanged, false);
});
});
context('#contentStyle', function() {
it('sets max-height if collapsed and overflowing', function() {
contentHeight = 200;
monitor.check();
assert.deepEqual(monitor.contentStyle(), { 'max-height': '100px' });
});
it('sets max height to empty if not overflowing', function() {
contentHeight = 80;
monitor.check();
assert.deepEqual(monitor.contentStyle(), { 'max-height': '' });
});
it('sets max-height if overflow state is unknown', function() {
// Before the initial overflow check, the state is unknown
assert.deepEqual(monitor.contentStyle(), { 'max-height': '100px' });
});
});
context('#check', function() {
it('calls onOverflowChanged() if state changed', function() {
contentHeight = 200;
monitor.check();
ctrl.onOverflowChanged = sinon.stub();
contentHeight = 250;
monitor.check();
assert.notCalled(ctrl.onOverflowChanged);
});
});
});
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
.excerpt { .excerpt {
transition: max-height $expand-duration ease-in; transition: max-height $expand-duration ease-in;
overflow: hidden; overflow: hidden;
position: relative;
} }
// a container which wraps the <excerpt> and contains the excerpt // a container which wraps the <excerpt> and contains the excerpt
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment