Unverified Commit 418f2d85 authored by Lyza Gardner's avatar Lyza Gardner Committed by GitHub

Merge pull request #1597 from hypothesis/icon-button

Add and use new `IconButton` component
parents 8377b378 d920ef48
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16" aria-hidden="true" focusable="false" class="Icon Icon--profile"><g fill-rule="evenodd"><rect fill="none" stroke="none" x="0" y="0" width="16" height="16"></rect><path fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M1 15c0-2.761 3.134-5 7-5s7 2.239 7 5M8 7a3 3 0 1 1 0-6 3 3 0 0 1 0 6z"></path></g></svg>
<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg width="16px" height="16px" viewBox="0 0 16 16" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:sketch="http://www.bohemiancoding.com/sketch/ns">
<!-- Generator: Sketch 3.3.3 (12072) - http://www.bohemiancoding.com/sketch -->
<title>Artboard 1 Copy 2</title>
<desc>Created with Sketch.</desc>
<defs></defs>
<g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd" sketch:type="MSPage">
<g id="Artboard-1-Copy-2" sketch:type="MSArtboardGroup" fill="#000000">
<g id="Shape-+-Shape-Copy-28" sketch:type="MSLayerGroup">
<path d="M6,10 L6,10 C8.209139,10 10,8.209139 10,6 C10,3.790861 8.209139,2 6,2 C3.790861,2 2,3.790861 2,6 C2,8.209139 3.790861,10 6,10 L6,10 Z M6,12 L6,12 C2.6862915,12 0,9.3137085 0,6 C0,2.6862915 2.6862915,0 6,0 C9.3137085,0 12,2.6862915 12,6 C12,9.3137085 9.3137085,12 6,12 L6,12 Z" id="Shape" sketch:type="MSShapeGroup"></path>
</g>
<path d="M9.29289322,10.7071068 L14.2426407,15.6568542 L15.6568542,14.2426407 L10.7071068,9.29289322 L9.29289322,10.7071068 L9.29289322,10.7071068 Z" id="Shape" sketch:type="MSShapeGroup"></path>
</g>
</g>
</svg>
\ No newline at end of file
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16" aria-hidden="true" focusable="false" class="Icon Icon--search"><g fill-rule="evenodd"><rect fill="none" stroke="none" x="0" y="0" width="16" height="16"></rect><path fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9.536 9.536a5 5 0 1 1-7.071-7.071 5 5 0 0 1 7.07 7.07L15 15 9.536 9.536z"></path></g></svg>
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16" aria-hidden="true" focusable="false" class="Icon Icon--sort"><g fill-rule="evenodd"><rect fill="none" stroke="none" x="0" y="0" width="16" height="16"></rect><path fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M5 9V2v7zM1 5l4-4 4 4m2 2v7-7zm-4 4l4 4 4-4"></path></g></svg>
......@@ -6,11 +6,11 @@ const { createElement } = require('preact');
const { withServices } = require('../util/service-context');
const { isShareable, shareURI } = require('../util/annotation-sharing');
const AnnotationActionButton = require('./annotation-action-button');
const AnnotationShareControl = require('./annotation-share-control');
const IconButton = require('./icon-button');
/**
* A collection of `AnnotationActionButton`s in the footer area of an annotation.
* A collection of `IconButton`s in the footer area of an annotation.
*/
function AnnotationActionBar({
annotation,
......@@ -45,16 +45,12 @@ function AnnotationActionBar({
return (
<div className="annotation-action-bar">
{showEditAction && (
<AnnotationActionButton icon="edit" label="Edit" onClick={onEdit} />
<IconButton icon="edit" title="Edit" onClick={onEdit} />
)}
{showDeleteAction && (
<AnnotationActionButton
icon="trash"
label="Delete"
onClick={onDelete}
/>
<IconButton icon="trash" title="Delete" onClick={onDelete} />
)}
<AnnotationActionButton icon="reply" label="Reply" onClick={onReply} />
<IconButton icon="reply" title="Reply" onClick={onReply} />
{showShareAction && (
<AnnotationShareControl
group={annotationGroup}
......@@ -63,17 +59,17 @@ function AnnotationActionBar({
/>
)}
{showFlagAction && !annotation.flagged && (
<AnnotationActionButton
<IconButton
icon="flag"
label="Report this annotation to moderators"
title="Report this annotation to moderators"
onClick={onFlag}
/>
)}
{showFlagAction && annotation.flagged && (
<AnnotationActionButton
<IconButton
isActive={true}
icon="flag--active"
label="Annotation has been reported to the moderators"
title="Annotation has been reported to the moderators"
/>
)}
</div>
......
......@@ -8,7 +8,7 @@ const useElementShouldClose = require('./hooks/use-element-should-close');
const { copyText } = require('../util/copy-to-clipboard');
const { withServices } = require('../util/service-context');
const AnnotationActionButton = require('./annotation-action-button');
const IconButton = require('./icon-button');
const ShareLinks = require('./share-links');
const SvgIcon = require('./svg-icon');
......@@ -84,12 +84,7 @@ function AnnotationShareControl({
return (
<div className="annotation-share-control" ref={shareRef}>
<AnnotationActionButton
icon="share"
isDisabled={false}
label="Share"
onClick={toggleSharePanel}
/>
<IconButton icon="share" title="Share" onClick={toggleSharePanel} />
{isOpen && (
<div className="annotation-share-panel">
<div className="annotation-share-panel__header">
......
......@@ -7,37 +7,47 @@ const { createElement } = require('preact');
const SvgIcon = require('./svg-icon');
/**
* A simple icon-only button for actions applicable to annotations
* A simple icon-only button
*/
function AnnotationActionButton({
function IconButton({
className = '',
icon,
isActive = false,
label,
title,
onClick = () => null,
useCompactStyle = false,
}) {
return (
<button
className={classnames('annotation-action-button', {
className={classnames('icon-button', className, {
'is-active': isActive,
'icon-button--compact': useCompactStyle,
})}
onClick={onClick}
aria-label={label}
title={label}
aria-pressed={isActive}
title={title}
>
<SvgIcon name={icon} className="annotation-action-button__icon" />
<SvgIcon name={icon} className="icon-button__icon" />
</button>
);
}
AnnotationActionButton.propTypes = {
IconButton.propTypes = {
/** Optional additional class(es) to apply to the component element
* NB: Padding is controlled by the component's styles. Use
* `useCompactStyle` for tighter padding.
*/
className: propTypes.string,
/** The name of the SVGIcon to render */
icon: propTypes.string.isRequired,
/** Is this button currently in an "active" or "on" state? */
isActive: propTypes.bool,
/** a label used for the `title` and `aria-label` attributes */
label: propTypes.string.isRequired,
/** a value used for the `title` and `aria-label` attributes */
title: propTypes.string.isRequired,
/** optional callback for clicks */
onClick: propTypes.func,
/** tighten padding and make icon button fit in smaller space */
useCompactStyle: propTypes.bool,
};
module.exports = AnnotationActionButton;
module.exports = IconButton;
......@@ -7,6 +7,7 @@ const propTypes = require('prop-types');
const useStore = require('../store/use-store');
const IconButton = require('./icon-button');
const Spinner = require('./spinner');
/**
......@@ -60,14 +61,13 @@ function SearchInput({ alwaysExpanded, query, onSearch }) {
onInput={e => setPendingQuery(e.target.value)}
/>
{!isLoading && (
<button
type="button"
className="search-input__icon top-bar__btn"
title="Search"
<IconButton
className="search-input__icon-button top-bar__icon-button"
icon="search"
onClick={() => input.current.focus()}
>
<i className="h-icon-search" />
</button>
title="Search annotations"
useCompactStyle
/>
)}
{isLoading && <Spinner className="top-bar__btn" title="Loading…" />}
</form>
......
......@@ -4,6 +4,7 @@ const { createElement } = require('preact');
const useStore = require('../store/use-store');
const IconButton = require('./icon-button');
const Menu = require('./menu');
const MenuItem = require('./menu-item');
......@@ -35,7 +36,14 @@ function SortMenu() {
);
});
const menuLabel = <i className="h-icon-sort top-bar__btn" />;
const menuLabel = (
<IconButton
className="top-bar__icon-button"
icon="sort"
title="Sort annotations"
useCompactStyle
/>
);
return (
<div className="sort-menu">
......
......@@ -39,10 +39,13 @@ const icons = {
lock: require('../../images/icons/lock.svg'),
logo: require('../../images/icons/logo.svg'),
pointer: require('../../images/icons/pointer.svg'),
profile: require('../../images/icons/profile.svg'),
public: require('../../images/icons/public.svg'),
refresh: require('../../images/icons/refresh.svg'),
reply: require('../../images/icons/reply.svg'),
search: require('../../images/icons/search.svg'),
share: require('../../images/icons/share.svg'),
sort: require('../../images/icons/sort.svg'),
trash: require('../../images/icons/trash.svg'),
twitter: require('../../images/icons/twitter.svg'),
};
......
......@@ -52,7 +52,7 @@ describe('AnnotationActionBar', () => {
};
const getButton = (wrapper, iconName) => {
return wrapper.find('AnnotationActionButton').filter({ icon: iconName });
return wrapper.find('IconButton').filter({ icon: iconName });
};
beforeEach(() => {
......
......@@ -33,7 +33,7 @@ describe('AnnotationShareControl', () => {
function openElement(wrapper) {
act(() => {
wrapper
.find('AnnotationActionButton')
.find('IconButton')
.props()
.onClick();
});
......@@ -92,7 +92,7 @@ describe('AnnotationShareControl', () => {
act(() => {
wrapper
.find('AnnotationActionButton')
.find('IconButton')
.props()
.onClick();
});
......
......@@ -3,18 +3,18 @@
const { createElement } = require('preact');
const { mount } = require('enzyme');
const AnnotationActionButton = require('../annotation-action-button');
const IconButton = require('../icon-button');
const mockImportedComponents = require('./mock-imported-components');
describe('AnnotationActionButton', () => {
describe('IconButton', () => {
let fakeOnClick;
function createComponent(props = {}) {
return mount(
<AnnotationActionButton
<IconButton
icon="fakeIcon"
isDisabled={false}
label="My Action"
isActive={false}
title="My Action"
onClick={fakeOnClick}
{...props}
/>
......@@ -23,11 +23,11 @@ describe('AnnotationActionButton', () => {
beforeEach(() => {
fakeOnClick = sinon.stub();
AnnotationActionButton.$imports.$mock(mockImportedComponents());
IconButton.$imports.$mock(mockImportedComponents());
});
afterEach(() => {
AnnotationActionButton.$imports.$restore();
IconButton.$imports.$restore();
});
it('adds active className if `isActive` is `true`', () => {
......@@ -36,14 +36,31 @@ describe('AnnotationActionButton', () => {
assert.isTrue(wrapper.find('button').hasClass('is-active'));
});
it('renders `SvgIcon` if icon property set', () => {
it('renders `SvgIcon` for associated icon', () => {
const wrapper = createComponent();
assert.equal(wrapper.find('SvgIcon').prop('name'), 'fakeIcon');
});
it('sets ARIA `aria-pressed` attribute if `isActive`', () => {
const wrapper = createComponent({ isActive: true });
assert.isTrue(wrapper.find('button').prop('aria-pressed'));
});
it('invokes `onClick` callback when pressed', () => {
const wrapper = createComponent();
wrapper.find('button').simulate('click');
assert.calledOnce(fakeOnClick);
});
it('adds additional class name passed in `className` prop', () => {
const wrapper = createComponent({ className: 'my-class' });
assert.isTrue(wrapper.hasClass('my-class'));
});
it('sets compact style if `useCompactStyle` is set`', () => {
const wrapper = createComponent({ useCompactStyle: true });
assert.isTrue(wrapper.find('button').hasClass('icon-button--compact'));
});
});
......@@ -54,12 +54,9 @@ describe('TopBar', () => {
TopBar.$imports.$restore();
});
function applyUpdateBtn(wrapper) {
return wrapper.find('.top-bar__btn--refresh');
}
function helpBtn(wrapper) {
return wrapper.find('.top-bar__help-btn');
// Helper to retrieve an `IconButton` by icon name, for convenience
function getButton(wrapper, iconName) {
return wrapper.find('IconButton').filter({ icon: iconName });
}
function createTopBar(props = {}) {
......@@ -79,21 +76,23 @@ describe('TopBar', () => {
it('shows the pending update count', () => {
fakeStore.pendingUpdateCount.returns(1);
const wrapper = createTopBar();
const applyBtn = applyUpdateBtn(wrapper);
const applyBtn = getButton(wrapper, 'refresh');
assert.isTrue(applyBtn.exists());
});
it('does not show the pending update count when there are no updates', () => {
const wrapper = createTopBar();
const applyBtn = applyUpdateBtn(wrapper);
const applyBtn = getButton(wrapper, 'refresh');
assert.isFalse(applyBtn.exists());
});
it('applies updates when clicked', () => {
fakeStore.pendingUpdateCount.returns(1);
const wrapper = createTopBar();
const applyBtn = applyUpdateBtn(wrapper);
applyBtn.simulate('click');
const applyBtn = getButton(wrapper, 'refresh');
applyBtn.props().onClick();
assert.called(fakeStreamer.applyPendingUpdates);
});
......@@ -101,8 +100,10 @@ describe('TopBar', () => {
context('no help service handler configured in services (default)', () => {
it('toggles Help Panel on click', () => {
const wrapper = createTopBar();
const help = helpBtn(wrapper);
help.simulate('click');
const helpButton = getButton(wrapper, 'help');
helpButton.props().onClick();
assert.calledWith(fakeStore.toggleSidebarPanel, uiConstants.PANEL_HELP);
});
......@@ -114,20 +115,22 @@ describe('TopBar', () => {
},
});
const wrapper = createTopBar();
const help = helpBtn(wrapper);
const helpButton = getButton(wrapper, 'help');
wrapper.update();
assert.isTrue(help.hasClass('is-active'));
assert.isOk(help.prop('aria-pressed'));
assert.isTrue(helpButton.props().isActive);
});
context('help service handler configured in services', () => {
it('fires a bridge event if help clicked and service is configured', () => {
fakeServiceConfig.returns({ onHelpRequestProvided: true });
const wrapper = createTopBar();
const help = helpBtn(wrapper);
help.simulate('click');
const helpButton = getButton(wrapper, 'help');
helpButton.props().onClick();
assert.equal(fakeStore.toggleSidebarPanel.callCount, 0);
assert.calledWith(fakeBridge.call, bridgeEvents.HELP_REQUESTED);
});
......@@ -208,7 +211,10 @@ describe('TopBar', () => {
it('toggles the share annotations panel when "Share" is clicked', () => {
const wrapper = createTopBar();
wrapper.find('.top-bar__share-btn').simulate('click');
const shareButton = getButton(wrapper, 'share');
shareButton.props().onClick();
assert.calledWith(
fakeStore.toggleSidebarPanel,
uiConstants.PANEL_SHARE_ANNOTATIONS
......@@ -221,11 +227,10 @@ describe('TopBar', () => {
activePanelName: uiConstants.PANEL_SHARE_ANNOTATIONS,
},
});
const wrapper = createTopBar();
const shareEl = wrapper.find('.top-bar__share-btn');
const shareButton = getButton(wrapper, 'share');
assert.include(shareEl.prop('className'), 'is-active');
assert.isTrue(shareButton.prop('isActive'));
});
it('displays search input in the sidebar', () => {
......
......@@ -13,48 +13,12 @@ const { withServices } = require('../util/service-context');
const uiConstants = require('../ui-constants');
const GroupList = require('./group-list');
const IconButton = require('./icon-button');
const SearchInput = require('./search-input');
const StreamSearchInput = require('./stream-search-input');
const SortMenu = require('./sort-menu');
const SvgIcon = require('./svg-icon');
const UserMenu = require('./user-menu');
/**
* Reusable component to render a button for toggling a sidebar panel.
* Takes an `onClick` callback, as panel toggles are sometimes more complex than
* just opening and closing a panel.
*/
function TogglePanelButton({ panelName, iconName, title, onClick }) {
const currentActivePanel = useStore(
store => store.getState().sidebarPanels.activePanelName
);
const isActive = currentActivePanel === panelName;
return (
<button
className={classnames('top-bar__btn', `top-bar__${iconName}-btn`, {
'is-active': isActive,
})}
onClick={onClick}
title={title}
aria-pressed={isActive ? 'true' : 'false'}
>
<SvgIcon
name={iconName}
className={classnames(`top-bar__{$iconName}-icon`)}
/>
</button>
);
}
TogglePanelButton.propTypes = {
/** The panel's string name as defined in `uiConstants` */
panelName: propTypes.string.isRequired,
iconName: propTypes.string.isRequired,
title: propTypes.string.isRequired,
/** callback */
onClick: propTypes.func.isRequired,
};
/**
* The toolbar which appears at the top of the sidebar providing actions
* to switch groups, view account information, sort/filter annotations etc.
......@@ -86,6 +50,10 @@ function TopBar({
togglePanelFn(uiConstants.PANEL_SHARE_ANNOTATIONS);
};
const currentActivePanel = useStore(
store => store.getState().sidebarPanels.activePanelName
);
/**
* Open the help panel, or, if a service callback is configured to handle
* help requests, fire a relevant event instead
......@@ -130,11 +98,13 @@ function TopBar({
<div className="top-bar__inner content">
<StreamSearchInput />
<div className="top-bar__expander" />
<TogglePanelButton
panelName={uiConstants.PANEL_HELP}
iconName="help"
title="Help"
<IconButton
className="top-bar__icon-button"
icon="help"
isActive={currentActivePanel === uiConstants.PANEL_HELP}
onClick={requestHelp}
title="Help"
useCompactStyle
/>
{loginControl}
</div>
......@@ -145,31 +115,37 @@ function TopBar({
<GroupList className="GroupList" auth={auth} />
<div className="top-bar__expander" />
{pendingUpdateCount > 0 && (
<button
className="top-bar__btn top-bar__btn--refresh"
<IconButton
className="top-bar__icon-button top-bar__icon-button--refresh"
icon="refresh"
onClick={applyPendingUpdates}
title={`Show ${pendingUpdateCount} new/updated ${
pendingUpdateCount === 1 ? 'annotation' : 'annotations'
}`}
>
<SvgIcon className="top-bar__apply-icon" name="refresh" />
</button>
useCompactStyle
/>
)}
<SearchInput query={filterQuery} onSearch={setFilterQuery} />
<SortMenu />
{showSharePageButton && (
<TogglePanelButton
panelName={uiConstants.PANEL_SHARE_ANNOTATIONS}
iconName="share"
title="Share annotations on this page"
<IconButton
className="top-bar__icon-button"
icon="share"
isActive={
currentActivePanel === uiConstants.PANEL_SHARE_ANNOTATIONS
}
onClick={toggleSharePanel}
title="Share annotations on this page"
useCompactStyle
/>
)}
<TogglePanelButton
panelName={uiConstants.PANEL_HELP}
iconName="help"
title="Help"
<IconButton
className="top-bar__icon-button"
icon="help"
isActive={currentActivePanel === uiConstants.PANEL_HELP}
onClick={requestHelp}
title="Help"
useCompactStyle
/>
{loginControl}
</div>
......
......@@ -8,6 +8,7 @@ const { isThirdPartyUser } = require('../util/account-id');
const serviceConfig = require('../service-config');
const { withServices } = require('../util/service-context');
const IconButton = require('./icon-button');
const Menu = require('./menu');
const MenuSection = require('./menu-section');
const MenuItem = require('./menu-item');
......@@ -44,7 +45,14 @@ function UserMenu({ auth, bridge, onLogout, serviceUrl, settings }) {
return props;
})();
const menuLabel = <i className="h-icon-account top-bar__btn" />;
const menuLabel = (
<IconButton
className="top-bar__icon-button"
icon="profile"
title="User menu"
useCompactStyle
/>
);
return (
<div className="user-menu">
<Menu label={menuLabel} title={auth.displayName} align="right">
......
@use "../../mixins/buttons";
@use "../../variables" as var;
.annotation-action-button {
.icon-button {
@include buttons.button-base;
&.is-active {
......@@ -11,11 +11,23 @@
color: var.$brand;
}
}
&--compact {
padding: 0.25em;
}
}
@media (pointer: coarse) {
.annotation-action-button {
.icon-button {
min-width: var.$touch-target-size;
min-height: var.$touch-target-size;
}
// Until the top bar can be refactored to allow for breathing room around
// the search interface, we can't spare the room for comfortable tap targets
// on touchscreen devices. This overrides `IconButton`'s larger tap targets.
.icon-button--compact {
min-width: auto;
min-height: auto;
}
}
......@@ -9,7 +9,7 @@
color: var.$grey-mid;
}
.search-input__icon {
.search-input__icon-button {
order: 0;
}
......@@ -27,11 +27,8 @@
width: 100%;
// The search box expands when focused, via a change in the
// `max-width` property. In Safari, the <input> will not accept
// focus if `max-width` is set to 0px so we set it to
// a near-zero positive value instead.
// See https://github.com/hypothesis/h/issues/2654
max-width: 0.1px;
// `max-width` property.
max-width: 0px;
transition: max-width 0.3s ease-out, padding-left 0.3s ease-out;
......
......@@ -16,6 +16,24 @@
// Force top-bar onto a new compositor layer so that it does not judder when
// the window is scrolled.
transform: translate3d(0, 0, 0);
&__icon-button {
&.is-active {
color: var.$grey-6;
&:hover {
color: var.$grey-6;
}
}
}
&__icon-button--refresh {
color: var.$brand;
&:hover {
color: var.$brand;
}
}
}
.top-bar--theme-clean {
......@@ -55,29 +73,3 @@
.top-bar__expander {
flex-grow: 1;
}
.top-bar__btn {
@include buttons.reset-native-btn-styles;
height: 100%;
color: var.$grey-semi;
display: inline-block;
cursor: pointer;
padding: 1px 3px 0 3px;
&:hover {
color: var.$grey-mid;
}
&.is-active {
color: var.$grey-mid;
}
&--refresh {
color: var.$brand;
&:hover {
color: var.$brand;
}
}
}
......@@ -26,7 +26,6 @@
// ----------
@use './components/action-button';
@use './components/annotation-action-bar';
@use './components/annotation-action-button';
@use './components/annotation';
@use './components/annotation-body';
@use './components/annotation-document-info';
......@@ -43,6 +42,7 @@
@use './components/group-list';
@use './components/group-list-item';
@use './components/help-panel';
@use './components/icon-button';
@use './components/logged-out-message';
@use './components/markdown-editor';
@use './components/markdown-view';
......
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