Unverified Commit 2364c416 authored by Lyza Gardner's avatar Lyza Gardner Committed by GitHub

Merge pull request #1782 from hypothesis/annotation-header-a11y

Replace link with `Button` in `AnnotationHeader` (A11y)
parents 857cde8e 0d338deb
import { createElement } from 'preact'; import { createElement } from 'preact';
import propTypes from 'prop-types'; import propTypes from 'prop-types';
import { isHighlight } from '../util/annotation-metadata'; import { isHighlight, isReply } from '../util/annotation-metadata';
import AnnotationDocumentInfo from './annotation-document-info'; import AnnotationDocumentInfo from './annotation-document-info';
import AnnotationShareInfo from './annotation-share-info'; import AnnotationShareInfo from './annotation-share-info';
import AnnotationUser from './annotation-user'; import AnnotationUser from './annotation-user';
import Button from './button';
import SvgIcon from './svg-icon'; import SvgIcon from './svg-icon';
import Timestamp from './timestamp'; import Timestamp from './timestamp';
...@@ -20,6 +21,7 @@ export default function AnnotationHeader({ ...@@ -20,6 +21,7 @@ export default function AnnotationHeader({
onReplyCountClick, onReplyCountClick,
replyCount, replyCount,
showDocumentInfo, showDocumentInfo,
threadIsCollapsed,
}) { }) {
const annotationLink = annotation.links ? annotation.links.html : ''; const annotationLink = annotation.links ? annotation.links.html : '';
const replyPluralized = !replyCount || replyCount > 1 ? 'replies' : 'reply'; const replyPluralized = !replyCount || replyCount > 1 ? 'replies' : 'reply';
...@@ -27,17 +29,21 @@ export default function AnnotationHeader({ ...@@ -27,17 +29,21 @@ export default function AnnotationHeader({
const hasBeenEdited = const hasBeenEdited =
annotation.updated && annotation.created !== annotation.updated; annotation.updated && annotation.created !== annotation.updated;
const showReplyButton = threadIsCollapsed && isReply(annotation);
const replyButtonText = `${replyCount} ${replyPluralized}`;
return ( return (
<header className="annotation-header"> <header className="annotation-header">
<div className="annotation-header__row"> <div className="annotation-header__row">
<AnnotationUser annotation={annotation} /> <AnnotationUser annotation={annotation} />
<div className="annotation-collapsed-replies"> {showReplyButton && (
{/* FIXME-A11Y */} <Button
{/* eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events, jsx-a11y/anchor-is-valid */} className="annotation-header__reply-toggle"
<a className="annotation-link" onClick={onReplyCountClick}> buttonText={replyButtonText}
{replyCount} {replyPluralized} onClick={onReplyCountClick}
</a> title="Expand replies"
</div> />
)}
{!isEditing && annotation.created && ( {!isEditing && annotation.created && (
<div className="annotation-header__timestamp"> <div className="annotation-header__timestamp">
{hasBeenEdited && ( {hasBeenEdited && (
...@@ -93,4 +99,8 @@ AnnotationHeader.propTypes = { ...@@ -93,4 +99,8 @@ AnnotationHeader.propTypes = {
* annotation and stream views * annotation and stream views
*/ */
showDocumentInfo: propTypes.bool, showDocumentInfo: propTypes.bool,
/**
* Is this thread currently collapsed?
*/
threadIsCollapsed: propTypes.bool.isRequired,
}; };
...@@ -102,6 +102,7 @@ function AnnotationOmega({ ...@@ -102,6 +102,7 @@ function AnnotationOmega({
onReplyCountClick={onReplyCountClick} onReplyCountClick={onReplyCountClick}
replyCount={replyCount} replyCount={replyCount}
showDocumentInfo={showDocumentInfo} showDocumentInfo={showDocumentInfo}
threadIsCollapsed={threadIsCollapsed}
/> />
{hasQuote && <AnnotationQuote annotation={annotation} />} {hasQuote && <AnnotationQuote annotation={annotation} />}
<AnnotationBody <AnnotationBody
......
...@@ -10,6 +10,7 @@ import mockImportedComponents from '../../../test-util/mock-imported-components' ...@@ -10,6 +10,7 @@ import mockImportedComponents from '../../../test-util/mock-imported-components'
describe('AnnotationHeader', () => { describe('AnnotationHeader', () => {
let fakeIsHighlight; let fakeIsHighlight;
let fakeIsReply;
const createAnnotationHeader = props => { const createAnnotationHeader = props => {
return mount( return mount(
...@@ -19,6 +20,7 @@ describe('AnnotationHeader', () => { ...@@ -19,6 +20,7 @@ describe('AnnotationHeader', () => {
onReplyCountClick={sinon.stub()} onReplyCountClick={sinon.stub()}
replyCount={0} replyCount={0}
showDocumentInfo={false} showDocumentInfo={false}
threadIsCollapsed={false}
{...props} {...props}
/> />
); );
...@@ -26,11 +28,13 @@ describe('AnnotationHeader', () => { ...@@ -26,11 +28,13 @@ describe('AnnotationHeader', () => {
beforeEach(() => { beforeEach(() => {
fakeIsHighlight = sinon.stub().returns(false); fakeIsHighlight = sinon.stub().returns(false);
fakeIsReply = sinon.stub().returns(false);
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({ $imports.$mock({
'../util/annotation-metadata': { '../util/annotation-metadata': {
isHighlight: fakeIsHighlight, isHighlight: fakeIsHighlight,
isReply: fakeIsReply,
}, },
}); });
}); });
...@@ -40,13 +44,38 @@ describe('AnnotationHeader', () => { ...@@ -40,13 +44,38 @@ describe('AnnotationHeader', () => {
}); });
describe('collapsed replies', () => { describe('collapsed replies', () => {
it('should have a callback', () => { const findReplyButton = wrapper =>
const fakeCallback = sinon.stub(); wrapper.find('Button').filter('.annotation-header__reply-toggle');
it('should render if annotation is a reply and thread is collapsed', () => {
let fakeCallback = sinon.stub();
fakeIsReply.returns(true);
const wrapper = createAnnotationHeader({ const wrapper = createAnnotationHeader({
onReplyCountClick: fakeCallback, onReplyCountClick: fakeCallback,
threadIsCollapsed: true,
});
const btn = findReplyButton(wrapper);
assert.isTrue(btn.exists());
assert.equal(btn.props().onClick, fakeCallback);
});
it('should not render if annotation is not a reply', () => {
fakeIsReply.returns(false);
const wrapper = createAnnotationHeader({
threadIsCollapsed: true,
});
const btn = findReplyButton(wrapper);
assert.isFalse(btn.exists());
});
it('should not render if thread is not collapsed', () => {
fakeIsReply.returns(true);
const wrapper = createAnnotationHeader({
threadIsCollapsed: false,
}); });
const replyCollapseLink = wrapper.find('.annotation-link'); const btn = findReplyButton(wrapper);
assert.equal(replyCollapseLink.prop('onClick'), fakeCallback); assert.isFalse(btn.exists());
}); });
[ [
...@@ -63,12 +92,14 @@ describe('AnnotationHeader', () => { ...@@ -63,12 +92,14 @@ describe('AnnotationHeader', () => {
expected: '2 replies', expected: '2 replies',
}, },
].forEach(testCase => { ].forEach(testCase => {
it(`it should render the annotation reply count (${testCase.replyCount})`, () => { it(`it should render the annotation reply count button (${testCase.replyCount})`, () => {
fakeIsReply.returns(true);
const wrapper = createAnnotationHeader({ const wrapper = createAnnotationHeader({
replyCount: testCase.replyCount, replyCount: testCase.replyCount,
threadIsCollapsed: true,
}); });
const replyCollapseLink = wrapper.find('.annotation-link'); const replyCollapseButton = findReplyButton(wrapper);
assert.equal(replyCollapseLink.text(), testCase.expected); assert.equal(replyCollapseButton.props().buttonText, testCase.expected);
}); });
}); });
}); });
......
...@@ -8,7 +8,8 @@ ...@@ -8,7 +8,8 @@
is-editing="vm.editing()" is-editing="vm.editing()"
on-reply-count-click="vm.onReplyCountClick()" on-reply-count-click="vm.onReplyCountClick()"
reply-count="vm.replyCount" reply-count="vm.replyCount"
show-document-info="vm.showDocumentInfo"> show-document-info="vm.showDocumentInfo"
thread-is-collapsed="vm.isCollapsed">
</annotation-header> </annotation-header>
<annotation-quote annotation="vm.annotation" ng-if="vm.quote()"> <annotation-quote annotation="vm.annotation" ng-if="vm.quote()">
......
...@@ -13,6 +13,16 @@ ...@@ -13,6 +13,16 @@
align-items: baseline; align-items: baseline;
} }
&__reply-toggle.button {
padding: 0 0.5em;
font-weight: 400;
background-color: transparent;
&:hover {
background-color: transparent;
color: var.$link-color-hover;
}
}
// Timestamps are right aligned in a flex row // Timestamps are right aligned in a flex row
&__timestamp { &__timestamp {
margin-left: auto; margin-left: auto;
......
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