Commit c827754c authored by Robert Knight's avatar Robert Knight

Add "Show" button to adder toolbar

Add a button to the adder toolbar to show the existing annotations that
relate to the selected text, along with a keyboard shortcut (the "s"
key) to trigger it. This provides a way to filter annotations in the
sidebar to those that correspond to a particular highlight or region of
text using only the keyboard.

Instead of an icon the "Show" toolbar button displays a badge whose
label is the number of annotations associated with the selection, as
that is more informative.

In implementing this, the CSS for the adder toolbar was refactored to
make the effects of several rules easier to follow and improve readability of
toolbar button labels:

 - Use of `!important` was removed. This used to be for the purpose of
   overriding any page styles which affected the adder. That issue no
   longer applies in most browsers due to the use of Shadow DOM to
   isolate the adder from the page's styles.

   This may cause a regression on some pages in older browsers which don't
   support Shadow DOM, if those pages' have generic CSS rules which
   style generic elements.

 - To make the label easier to read, the font size was slightly
   increased
parent 7032092c
...@@ -51,6 +51,14 @@ function nearestPositionedAncestor(el) { ...@@ -51,6 +51,14 @@ function nearestPositionedAncestor(el) {
return parentEl; return parentEl;
} }
/**
* @typedef AdderOptions
* @prop {() => any} onAnnotate - Callback invoked when "Annotate" button is clicked
* @prop {() => any} onHighlight - Callback invoked when "Highlight" button is clicked
* @prop {(annotations: Object[]) => any} onShowAnnotations -
* Callback invoked when "Show" button is clicked
*/
/** /**
* Container for the 'adder' toolbar which provides controls for the user to * Container for the 'adder' toolbar which provides controls for the user to
* annotate and highlight the selected text. * annotate and highlight the selected text.
...@@ -66,8 +74,8 @@ export class Adder { ...@@ -66,8 +74,8 @@ export class Adder {
* *
* The adder is initially hidden. * The adder is initially hidden.
* *
* @param {Element} container - The DOM element into which the adder will be created * @param {HTMLElement} container - The DOM element into which the adder will be created
* @param {Object} options - Options object specifying `onAnnotate` and `onHighlight` * @param {AdderOptions} options - Options object specifying `onAnnotate` and `onHighlight`
* event handlers. * event handlers.
*/ */
constructor(container, options) { constructor(container, options) {
...@@ -98,6 +106,17 @@ export class Adder { ...@@ -98,6 +106,17 @@ export class Adder {
this._arrowDirection = 'up'; this._arrowDirection = 'up';
this._onAnnotate = options.onAnnotate; this._onAnnotate = options.onAnnotate;
this._onHighlight = options.onHighlight; this._onHighlight = options.onHighlight;
this._onShowAnnotations = options.onShowAnnotations;
/**
* Annotation objects associated with the current selection. If non-empty,
* a "Show" button appears in the toolbar. Clicking the button calls the
* `onShowAnnotations` callback with the current value of `annotationsForSelection`.
*
* @type {Object[]}
*/
this.annotationsForSelection = [];
this._render(); this._render();
} }
...@@ -198,15 +217,18 @@ export class Adder { ...@@ -198,15 +217,18 @@ export class Adder {
switch (command) { switch (command) {
case 'annotate': case 'annotate':
this._onAnnotate(); this._onAnnotate();
this.hide();
break; break;
case 'highlight': case 'highlight':
this._onHighlight(); this._onHighlight();
this.hide();
break;
case 'show':
this._onShowAnnotations(this.annotationsForSelection);
break; break;
default: default:
break; break;
} }
this.hide();
}; };
render( render(
...@@ -214,6 +236,7 @@ export class Adder { ...@@ -214,6 +236,7 @@ export class Adder {
isVisible={this._isVisible} isVisible={this._isVisible}
arrowDirection={this._arrowDirection} arrowDirection={this._arrowDirection}
onCommand={handleCommand} onCommand={handleCommand}
annotationCount={this.annotationsForSelection.length}
/>, />,
this._shadowRoot this._shadowRoot
); );
......
...@@ -5,7 +5,7 @@ import propTypes from 'prop-types'; ...@@ -5,7 +5,7 @@ import propTypes from 'prop-types';
import { useShortcut } from '../../shared/shortcut'; import { useShortcut } from '../../shared/shortcut';
import SvgIcon from '../../shared/components/svg-icon'; import SvgIcon from '../../shared/components/svg-icon';
function ToolbarButton({ icon, label, onClick, shortcut }) { function ToolbarButton({ badgeCount, icon, label, onClick, shortcut }) {
useShortcut(shortcut, onClick); useShortcut(shortcut, onClick);
const title = shortcut ? `${label} (${shortcut})` : label; const title = shortcut ? `${label} (${shortcut})` : label;
...@@ -16,14 +16,18 @@ function ToolbarButton({ icon, label, onClick, shortcut }) { ...@@ -16,14 +16,18 @@ function ToolbarButton({ icon, label, onClick, shortcut }) {
onClick={onClick} onClick={onClick}
title={title} title={title}
> >
<SvgIcon name={icon} /> {icon && <SvgIcon name={icon} />}
{typeof badgeCount === 'number' && (
<span className="annotator-adder-actions__badge">{badgeCount}</span>
)}
<span className="annotator-adder-actions__label">{label}</span> <span className="annotator-adder-actions__label">{label}</span>
</button> </button>
); );
} }
ToolbarButton.propTypes = { ToolbarButton.propTypes = {
icon: propTypes.string.isRequired, badgeCount: propTypes.number,
icon: propTypes.string,
label: propTypes.string.isRequired, label: propTypes.string.isRequired,
onClick: propTypes.func.isRequired, onClick: propTypes.func.isRequired,
shortcut: propTypes.string, shortcut: propTypes.string,
...@@ -33,7 +37,12 @@ ToolbarButton.propTypes = { ...@@ -33,7 +37,12 @@ ToolbarButton.propTypes = {
* The toolbar that is displayed above selected text in the document providing * The toolbar that is displayed above selected text in the document providing
* options to create annotations or highlights. * options to create annotations or highlights.
*/ */
export default function AdderToolbar({ arrowDirection, isVisible, onCommand }) { export default function AdderToolbar({
arrowDirection,
isVisible,
onCommand,
annotationCount = 0,
}) {
const handleCommand = (event, command) => { const handleCommand = (event, command) => {
event.preventDefault(); event.preventDefault();
event.stopPropagation(); event.stopPropagation();
...@@ -46,6 +55,7 @@ export default function AdderToolbar({ arrowDirection, isVisible, onCommand }) { ...@@ -46,6 +55,7 @@ export default function AdderToolbar({ arrowDirection, isVisible, onCommand }) {
// the shortcut. This avoids conflicts with browser/OS shortcuts. // the shortcut. This avoids conflicts with browser/OS shortcuts.
const annotateShortcut = isVisible ? 'a' : null; const annotateShortcut = isVisible ? 'a' : null;
const highlightShortcut = isVisible ? 'h' : null; const highlightShortcut = isVisible ? 'h' : null;
const showShortcut = isVisible ? 's' : null;
// nb. The adder is hidden using the `visibility` property rather than `display` // nb. The adder is hidden using the `visibility` property rather than `display`
// so that we can compute its size in order to position it before display. // so that we can compute its size in order to position it before display.
...@@ -71,6 +81,17 @@ export default function AdderToolbar({ arrowDirection, isVisible, onCommand }) { ...@@ -71,6 +81,17 @@ export default function AdderToolbar({ arrowDirection, isVisible, onCommand }) {
label="Highlight" label="Highlight"
shortcut={highlightShortcut} shortcut={highlightShortcut}
/> />
{annotationCount > 0 && (
<div className="annotator-adder-actions__separator" />
)}
{annotationCount > 0 && (
<ToolbarButton
badgeCount={annotationCount}
onClick={e => handleCommand(e, 'show')}
label="Show"
shortcut={showShortcut}
/>
)}
</hypothesis-adder-actions> </hypothesis-adder-actions>
</hypothesis-adder-toolbar> </hypothesis-adder-toolbar>
); );
...@@ -90,8 +111,16 @@ AdderToolbar.propTypes = { ...@@ -90,8 +111,16 @@ AdderToolbar.propTypes = {
isVisible: propTypes.bool.isRequired, isVisible: propTypes.bool.isRequired,
/** /**
* Callback invoked with the name ("annotate", "highlight") of the selected * Callback invoked with the name ("annotate", "highlight", "show") of the
* command when a toolbar command is clicked. * selected command when a toolbar command is clicked.
*/ */
onCommand: propTypes.func.isRequired, onCommand: propTypes.func.isRequired,
/**
* Number of annotations associated with the selected text.
*
* If non-zero, a "Show" button is displayed to allow the user to see the
* annotations that correspond to the selection.
*/
annotationCount: propTypes.number,
}; };
...@@ -20,6 +20,11 @@ animationPromise = (fn) -> ...@@ -20,6 +20,11 @@ animationPromise = (fn) ->
catch error catch error
reject(error) reject(error)
annotationsForSelection = () ->
selection = window.getSelection()
range = selection.getRangeAt(0)
return rangeUtil.itemsForRange(range, (node) -> $(node).data('annotation'))
# A selector which matches elements added to the DOM by Hypothesis (eg. for # A selector which matches elements added to the DOM by Hypothesis (eg. for
# highlights and annotation UI). # highlights and annotation UI).
# #
...@@ -68,6 +73,8 @@ module.exports = class Guest extends Delegator ...@@ -68,6 +73,8 @@ module.exports = class Guest extends Delegator
self.setVisibleHighlights(true) self.setVisibleHighlights(true)
self.createHighlight() self.createHighlight()
document.getSelection().removeAllRanges() document.getSelection().removeAllRanges()
onShowAnnotations: (anns) ->
self.selectAnnotations(anns)
}) })
this.selections = selections(document).subscribe this.selections = selections(document).subscribe
next: (range) -> next: (range) ->
...@@ -425,6 +432,7 @@ module.exports = class Guest extends Delegator ...@@ -425,6 +432,7 @@ module.exports = class Guest extends Delegator
.addClass('h-icon-annotate'); .addClass('h-icon-annotate');
{left, top, arrowDirection} = this.adderCtrl.target(focusRect, isBackwards) {left, top, arrowDirection} = this.adderCtrl.target(focusRect, isBackwards)
this.adderCtrl.annotationsForSelection = annotationsForSelection()
this.adderCtrl.showAt(left, top, arrowDirection) this.adderCtrl.showAt(left, top, arrowDirection)
_onClearSelection: () -> _onClearSelection: () ->
......
...@@ -129,3 +129,35 @@ export function selectionFocusRect(selection) { ...@@ -129,3 +129,35 @@ export function selectionFocusRect(selection) {
return textBoxes[textBoxes.length - 1]; return textBoxes[textBoxes.length - 1];
} }
} }
/**
* Retrieve a set of items associated with nodes in a given range.
*
* @template T
* @param {Range} range
* @param {(n: Node) => T} itemForNode - Callback returning the item for a given node
* @return {T[]} items
*/
export function itemsForRange(range, itemForNode) {
const checkedNodes = new Set();
const items = new Set();
forEachNodeInRange(range, node => {
let current = node;
while (current) {
if (checkedNodes.has(current)) {
break;
}
checkedNodes.add(current);
const item = itemForNode(current);
if (item) {
items.add(item);
}
current = current.parentNode;
}
});
return [...items];
}
...@@ -36,6 +36,7 @@ describe('Adder', () => { ...@@ -36,6 +36,7 @@ describe('Adder', () => {
adderCallbacks = { adderCallbacks = {
onAnnotate: sinon.stub(), onAnnotate: sinon.stub(),
onHighlight: sinon.stub(), onHighlight: sinon.stub(),
onShowAnnotations: sinon.stub(),
}; };
adderEl = document.createElement('div'); adderEl = document.createElement('div');
document.body.appendChild(adderEl); document.body.appendChild(adderEl);
...@@ -84,22 +85,57 @@ describe('Adder', () => { ...@@ -84,22 +85,57 @@ describe('Adder', () => {
}); });
describe('button handling', () => { describe('button handling', () => {
const getButton = label =>
getContent(adderCtrl).querySelector(`button[title^="${label}"]`);
const triggerShortcut = key =>
document.body.dispatchEvent(new KeyboardEvent('keydown', { key }));
const showAdder = () => {
// nb. `act` is necessary here to flush effect hooks in `AdderToolbar`
// which setup shortcut handlers.
act(() => {
adderCtrl.showAt(0, 0, ARROW_POINTING_UP);
});
};
it('calls onHighlight callback when Highlight button is clicked', () => { it('calls onHighlight callback when Highlight button is clicked', () => {
const highlightBtn = getContent(adderCtrl).querySelector( const highlightBtn = getButton('Highlight');
'button[title^="Highlight"]'
);
highlightBtn.dispatchEvent(new Event('click')); highlightBtn.dispatchEvent(new Event('click'));
assert.called(adderCallbacks.onHighlight); assert.called(adderCallbacks.onHighlight);
}); });
it('calls onAnnotate callback when Annotate button is clicked', () => { it('calls onAnnotate callback when Annotate button is clicked', () => {
const annotateBtn = getContent(adderCtrl).querySelector( const annotateBtn = getButton('Annotate');
'button[title^="Annotate"]'
);
annotateBtn.dispatchEvent(new Event('click')); annotateBtn.dispatchEvent(new Event('click'));
assert.called(adderCallbacks.onAnnotate); assert.called(adderCallbacks.onAnnotate);
}); });
it('does not show "Show" button if the selection has no annotations', () => {
showAdder();
assert.isNull(getButton('Show'));
});
it('shows the "Show" button if the selection has annotations', () => {
adderCtrl.annotationsForSelection = ['ann1', 'ann2'];
showAdder();
const showBtn = getButton('Show');
assert.ok(showBtn, '"Show" button not visible');
assert.equal(showBtn.querySelector('span').textContent, '2');
});
it('calls onShowAnnotations callback when Show button is clicked', () => {
adderCtrl.annotationsForSelection = ['ann1'];
showAdder();
const showBtn = getButton('Show');
showBtn.click();
assert.called(adderCallbacks.onShowAnnotations);
assert.calledWith(adderCallbacks.onShowAnnotations, ['ann1']);
});
it("calls onAnnotate callback when Annotate button's label is clicked", () => { it("calls onAnnotate callback when Annotate button's label is clicked", () => {
const annotateLabel = getContent(adderCtrl).querySelector( const annotateLabel = getContent(adderCtrl).querySelector(
'button[title^="Annotate"] > span' 'button[title^="Annotate"] > span'
...@@ -109,28 +145,32 @@ describe('Adder', () => { ...@@ -109,28 +145,32 @@ describe('Adder', () => {
}); });
it('calls onAnnotate callback when shortcut is pressed if adder is visible', () => { it('calls onAnnotate callback when shortcut is pressed if adder is visible', () => {
// nb. `act` is necessary here to flush effect hooks in `AdderToolbar`. showAdder();
act(() => { triggerShortcut('a');
adderCtrl.showAt(100, 100, ARROW_POINTING_UP);
});
document.body.dispatchEvent(new KeyboardEvent('keydown', { key: 'a' }));
assert.called(adderCallbacks.onAnnotate); assert.called(adderCallbacks.onAnnotate);
}); });
it('calls onHighlight callback when shortcut is pressed if adder is visible', () => { it('calls onHighlight callback when shortcut is pressed if adder is visible', () => {
// nb. `act` is necessary here to flush effect hooks in `AdderToolbar`. showAdder();
act(() => { triggerShortcut('h');
adderCtrl.showAt(100, 100, ARROW_POINTING_UP);
});
document.body.dispatchEvent(new KeyboardEvent('keydown', { key: 'h' }));
assert.called(adderCallbacks.onHighlight); assert.called(adderCallbacks.onHighlight);
}); });
it('calls onShowAnnotations callback when shortcut is pressed if adder is visible', () => {
adderCtrl.annotationsForSelection = ['ann1'];
showAdder();
triggerShortcut('s');
assert.called(adderCallbacks.onShowAnnotations);
});
it('does not call callbacks when adder is hidden', () => { it('does not call callbacks when adder is hidden', () => {
document.body.dispatchEvent(new KeyboardEvent('keydown', { key: 'a' })); triggerShortcut('a');
document.body.dispatchEvent(new KeyboardEvent('keydown', { key: 'h' })); triggerShortcut('h');
triggerShortcut('s');
assert.notCalled(adderCallbacks.onAnnotate); assert.notCalled(adderCallbacks.onAnnotate);
assert.notCalled(adderCallbacks.onHighlight); assert.notCalled(adderCallbacks.onHighlight);
assert.notCalled(adderCallbacks.onShowAnnotations);
}); });
}); });
......
...@@ -15,12 +15,13 @@ scrollIntoView = sinon.stub() ...@@ -15,12 +15,13 @@ scrollIntoView = sinon.stub()
class FakeAdder class FakeAdder
instance: null instance: null
constructor: -> constructor: (container, options) ->
FakeAdder::instance = this FakeAdder::instance = this
this.hide = sinon.stub() this.hide = sinon.stub()
this.showAt = sinon.stub() this.showAt = sinon.stub()
this.target = sinon.stub() this.target = sinon.stub()
this.options = options
class FakePlugin extends Plugin class FakePlugin extends Plugin
instance: null instance: null
...@@ -57,6 +58,7 @@ describe 'Guest', -> ...@@ -57,6 +58,7 @@ describe 'Guest', ->
FakeAdder::instance = null FakeAdder::instance = null
rangeUtil = { rangeUtil = {
itemsForRange: sinon.stub().returns([])
isSelectionBackwards: sinon.stub() isSelectionBackwards: sinon.stub()
selectionFocusRect: sinon.stub() selectionFocusRect: sinon.stub()
} }
...@@ -335,6 +337,17 @@ describe 'Guest', -> ...@@ -335,6 +337,17 @@ describe 'Guest', ->
assert.calledWith(guest.plugins.CrossFrame.call, 'hideSidebar') assert.calledWith(guest.plugins.CrossFrame.call, 'hideSidebar')
describe 'when the selection changes', -> describe 'when the selection changes', ->
container = null
beforeEach ->
container = document.createElement('div')
container.innerHTML = 'test text'
document.body.appendChild(container)
window.getSelection().selectAllChildren(container)
afterEach ->
container.remove()
it 'shows the adder if the selection contains text', -> it 'shows the adder if the selection contains text', ->
guest = createGuest() guest = createGuest()
rangeUtil.selectionFocusRect.returns({left: 0, top: 0, width: 5, height: 5}) rangeUtil.selectionFocusRect.returns({left: 0, top: 0, width: 5, height: 5})
...@@ -344,6 +357,22 @@ describe 'Guest', -> ...@@ -344,6 +357,22 @@ describe 'Guest', ->
selections.next({}) selections.next({})
assert.called FakeAdder::instance.showAt assert.called FakeAdder::instance.showAt
it 'sets the annotations associated with the selection', ->
guest = createGuest()
ann = {}
$(container).data('annotation', ann)
rangeUtil.selectionFocusRect.returns({left: 0, top: 0, width: 5, height: 5})
FakeAdder::instance.target.returns({
left: 0, top: 0, arrowDirection: adder.ARROW_POINTING_UP
})
rangeUtil.itemsForRange.callsFake((range, callback) ->
[callback(range.startContainer)]
)
selections.next({})
assert.deepEqual FakeAdder::instance.annotationsForSelection, [ann]
it 'hides the adder if the selection does not contain text', -> it 'hides the adder if the selection does not contain text', ->
guest = createGuest() guest = createGuest()
rangeUtil.selectionFocusRect.returns(null) rangeUtil.selectionFocusRect.returns(null)
...@@ -356,6 +385,17 @@ describe 'Guest', -> ...@@ -356,6 +385,17 @@ describe 'Guest', ->
selections.next(null) selections.next(null)
assert.called FakeAdder::instance.hide assert.called FakeAdder::instance.hide
describe 'when adder toolbar buttons are clicked', ->
# TODO - Add tests for "Annotate" and "Highlight" buttons.
it 'shows annotations if "Show" is clicked', ->
createGuest()
FakeAdder::instance.options.onShowAnnotations([{ $tag: 'ann1' }]);
assert.calledWith(fakeCrossFrame.call, 'showSidebar')
assert.calledWith(fakeCrossFrame.call, 'showAnnotations', ['ann1'])
describe '#getDocumentInfo()', -> describe '#getDocumentInfo()', ->
guest = null guest = null
......
...@@ -134,4 +134,21 @@ describe('annotator.range-util', function () { ...@@ -134,4 +134,21 @@ describe('annotator.range-util', function () {
); );
}); });
}); });
describe('itemsForRange', () => {
it('returns unique items for range', () => {
const range = document.createRange();
range.setStart(testNode, 0);
range.setEnd(testNode, testNode.childNodes.length);
const data = new Map();
data.set(testNode, 'itemA');
data.set(testNode.childNodes[0], 'itemB');
data.set(testNode.childNodes[1], 'itemB'); // Intentional duplicate.
data.set(testNode.childNodes[2], 'itemC');
const items = rangeUtil.itemsForRange(range, item => data.get(item));
assert.deepEqual(items, ['itemA', 'itemB', 'itemC']);
});
});
}); });
...@@ -97,20 +97,12 @@ $adder-transition-duration: 80ms; ...@@ -97,20 +97,12 @@ $adder-transition-duration: 80ms;
flex-direction: row; flex-direction: row;
} }
.annotator-adder-actions:hover .annotator-adder-actions__button {
color: var.$grey-semi !important;
}
.annotator-adder-actions:hover .annotator-adder-actions__label {
color: var.$grey-semi !important;
}
.annotator-adder-actions__button { .annotator-adder-actions__button {
box-shadow: none; box-shadow: none;
font-family: h; font-family: h;
font-size: 18px; font-size: 18px;
background: transparent !important; background: transparent;
color: var.$grey-mid !important; color: var.$grey-mid;
display: flex; display: flex;
flex-direction: column; flex-direction: column;
align-items: center; align-items: center;
...@@ -125,11 +117,20 @@ $adder-transition-duration: 80ms; ...@@ -125,11 +117,20 @@ $adder-transition-duration: 80ms;
transition: color $adder-transition-duration; transition: color $adder-transition-duration;
} }
.annotator-adder-actions .annotator-adder-actions__button:hover { .annotator-adder-actions__separator {
color: var.$grey-mid !important; margin: 5px 5px;
border-right: 1px solid var.$grey-4;
}
// The toolbar has full contrast when not hovered. When the toolbar is hovered,
// the buttons are dimmed except for the one which is currently hovered.
.annotator-adder-actions:hover {
.annotator-adder-actions__button:not(:hover) {
color: var.$grey-semi;
.annotator-adder-actions__label { .annotator-adder-actions__badge {
color: var.$grey-mid !important; background-color: var.$grey-semi;
}
} }
} }
...@@ -137,8 +138,20 @@ $adder-transition-duration: 80ms; ...@@ -137,8 +138,20 @@ $adder-transition-duration: 80ms;
margin-bottom: 2px; margin-bottom: 2px;
margin-top: 4px; margin-top: 4px;
font-size: 11px; font-size: 12px;
font-family: sans-serif; font-family: sans-serif;
color: var.$grey-mid !important;
transition: color $adder-transition-duration; transition: color $adder-transition-duration;
} }
.annotator-adder-actions__badge {
background-color: var.$grey-mid;
border-radius: 3px;
color: white;
// The badge should be vertically aligned with icons in other toolbar buttons
// and the label underneath should also align with labels in other buttons.
font-family: sans-serif;
font-size: 12px;
font-weight: bold;
padding: 2px 4px;
}
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