Commit 8bd23863 authored by Lyza Danger Gardner's avatar Lyza Danger Gardner

Convert icons and button in `SelectionTabs` and `NewNoteBtn`

- Convert `annotate` icon in `SelectionTabs` to SVG
- Remove redundant/misleading `note` icon from `SelectionTabs`
- Remove tutorial-display conditional from `SelectionTabs`
- Remove excess SASS rules from `SelectionTabs`
parent b66e69e8
...@@ -24,6 +24,7 @@ function Button({ ...@@ -24,6 +24,7 @@ function Button({
icon = '', icon = '',
isActive = false, isActive = false,
onClick = () => null, onClick = () => null,
style = {},
title, title,
useCompactStyle = false, useCompactStyle = false,
useInputStyle = false, useInputStyle = false,
...@@ -51,6 +52,7 @@ function Button({ ...@@ -51,6 +52,7 @@ function Button({
onClick={onClick} onClick={onClick}
aria-pressed={isActive} aria-pressed={isActive}
title={title} title={title}
style={style}
> >
{icon && <SvgIcon name={icon} className="button__icon" />} {icon && <SvgIcon name={icon} className="button__icon" />}
{buttonText} {buttonText}
...@@ -108,6 +110,9 @@ Button.propTypes = { ...@@ -108,6 +110,9 @@ Button.propTypes = {
/** callback for button clicks */ /** callback for button clicks */
onClick: propTypes.func, onClick: propTypes.func,
/** optional inline styling */
style: propTypes.object,
/** /**
* `title`, used for button `title`, is required unless `buttonText` is present * `title`, used for button `title`, is required unless `buttonText` is present
*/ */
......
...@@ -8,6 +8,8 @@ const useStore = require('../store/use-store'); ...@@ -8,6 +8,8 @@ const useStore = require('../store/use-store');
const { applyTheme } = require('../util/theme'); const { applyTheme } = require('../util/theme');
const { withServices } = require('../util/service-context'); const { withServices } = require('../util/service-context');
const Button = require('./button');
function NewNoteButton({ $rootScope, settings }) { function NewNoteButton({ $rootScope, settings }) {
const store = useStore(store => ({ const store = useStore(store => ({
frames: store.frames(), frames: store.frames(),
...@@ -23,13 +25,16 @@ function NewNoteButton({ $rootScope, settings }) { ...@@ -23,13 +25,16 @@ function NewNoteButton({ $rootScope, settings }) {
}; };
return ( return (
<button <div className="new-note-button">
style={applyTheme(['ctaBackgroundColor'], settings)} <Button
className="new-note__create" buttonText="New note"
onClick={onNewNoteBtnClick} icon="add"
> onClick={onNewNoteBtnClick}
+ New note style={applyTheme(['ctaBackgroundColor'], settings)}
</button> useCompactStyle
usePrimaryStyle
/>
</div>
); );
} }
NewNoteButton.propTypes = { NewNoteButton.propTypes = {
......
...@@ -6,11 +6,12 @@ const { createElement } = require('preact'); ...@@ -6,11 +6,12 @@ const { createElement } = require('preact');
const { Fragment } = require('preact'); const { Fragment } = require('preact');
const NewNoteBtn = require('./new-note-btn'); const NewNoteBtn = require('./new-note-btn');
const sessionUtil = require('../util/session');
const uiConstants = require('../ui-constants'); const uiConstants = require('../ui-constants');
const useStore = require('../store/use-store'); const useStore = require('../store/use-store');
const { withServices } = require('../util/service-context'); const { withServices } = require('../util/service-context');
const SvgIcon = require('./svg-icon');
/** /**
* Display name of the tab and annotation count. * Display name of the tab and annotation count.
*/ */
...@@ -72,7 +73,7 @@ Tab.propTypes = { ...@@ -72,7 +73,7 @@ Tab.propTypes = {
* Tabbed display of annotations and notes. * Tabbed display of annotations and notes.
*/ */
function SelectionTabs({ isLoading, settings, session }) { function SelectionTabs({ isLoading, settings }) {
const selectedTab = useStore(store => store.getState().selection.selectedTab); const selectedTab = useStore(store => store.getState().selection.selectedTab);
const noteCount = useStore(store => store.noteCount()); const noteCount = useStore(store => store.noteCount());
const annotationCount = useStore(store => store.annotationCount()); const annotationCount = useStore(store => store.annotationCount());
...@@ -101,10 +102,6 @@ function SelectionTabs({ isLoading, settings, session }) { ...@@ -101,10 +102,6 @@ function SelectionTabs({ isLoading, settings, session }) {
const showNotesUnavailableMessage = const showNotesUnavailableMessage =
selectedTab === uiConstants.TAB_NOTES && noteCount === 0; selectedTab === uiConstants.TAB_NOTES && noteCount === 0;
const showSidebarTutorial = sessionUtil.shouldShowSidebarTutorial(
session.state
);
return ( return (
<Fragment> <Fragment>
<div <div
...@@ -146,32 +143,23 @@ function SelectionTabs({ isLoading, settings, session }) { ...@@ -146,32 +143,23 @@ function SelectionTabs({ isLoading, settings, session }) {
{selectedTab === uiConstants.TAB_NOTES && {selectedTab === uiConstants.TAB_NOTES &&
settings.enableExperimentalNewNoteButton && <NewNoteBtn />} settings.enableExperimentalNewNoteButton && <NewNoteBtn />}
{!isLoading && ( {!isLoading && (
<div className="selection-tabs__empty-message"> <div>
{showNotesUnavailableMessage && ( {showNotesUnavailableMessage && (
<div className="annotation-unavailable-message"> <div className="selection-tabs__message">
<p className="annotation-unavailable-message__label"> There are no page notes in this group.
There are no page notes in this group.
{settings.enableExperimentalNewNoteButton &&
!showSidebarTutorial && (
<div className="annotation-unavailable-message__tutorial">
Create one by clicking the{' '}
<i className="help-icon h-icon-note" /> button.
</div>
)}
</p>
</div> </div>
)} )}
{showAnnotationsUnavailableMessage && ( {showAnnotationsUnavailableMessage && (
<div className="annotation-unavailable-message"> <div className="selection-tabs__message">
<p className="annotation-unavailable-message__label"> There are no annotations in this group.
There are no annotations in this group. <br />
{!showSidebarTutorial && ( Create one by selecting some text and clicking the{' '}
<div className="annotation-unavailable-message__tutorial"> <SvgIcon
Create one by selecting some text and clicking the{' '} name="annotate"
<i className="help-icon h-icon-annotate" /> button. inline="true"
</div> className="selection-tabs__icon"
)} />{' '}
</p> button.
</div> </div>
)} )}
</div> </div>
...@@ -187,9 +175,8 @@ SelectionTabs.propTypes = { ...@@ -187,9 +175,8 @@ SelectionTabs.propTypes = {
// Injected services. // Injected services.
settings: propTypes.object.isRequired, settings: propTypes.object.isRequired,
session: propTypes.object.isRequired,
}; };
SelectionTabs.injectedProps = ['session', 'settings']; SelectionTabs.injectedProps = ['settings'];
module.exports = withServices(SelectionTabs); module.exports = withServices(SelectionTabs);
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
const { mount } = require('enzyme'); const { mount } = require('enzyme');
const { createElement } = require('preact'); const { createElement } = require('preact');
const { act } = require('preact/test-utils');
const events = require('../../events'); const events = require('../../events');
const NewNoteButton = require('../new-note-btn'); const NewNoteButton = require('../new-note-btn');
...@@ -49,22 +50,22 @@ describe('NewNoteButton', function() { ...@@ -49,22 +50,22 @@ describe('NewNoteButton', function() {
NewNoteButton.$imports.$restore(); NewNoteButton.$imports.$restore();
}); });
it('creates the component', () => { it("sets a backgroundColor equal to the setting's ctaBackgroundColor color", () => {
const wrapper = createComponent(); const wrapper = createComponent();
assert.include(wrapper.text(), 'New note');
});
it("has a backgroundColor equal to the setting's ctaBackgroundColor color", () => {
const wrapper = createComponent().find('button');
assert.equal( assert.equal(
wrapper.prop('style').backgroundColor, wrapper.find('Button').prop('style').backgroundColor,
fakeSettings.branding.ctaBackgroundColor fakeSettings.branding.ctaBackgroundColor
); );
}); });
it('should broadcast BEFORE_ANNOTATION_CREATED event when the new note button is clicked', () => { it('should broadcast BEFORE_ANNOTATION_CREATED event when the new note button is clicked', () => {
const wrapper = createComponent(); const wrapper = createComponent();
wrapper.find('button').simulate('click'); act(() => {
wrapper
.find('Button')
.props()
.onClick();
});
const topLevelFrame = fakeStore.frames().find(f => !f.id); const topLevelFrame = fakeStore.frames().find(f => !f.id);
assert.calledWith( assert.calledWith(
fakeRootScope.$broadcast, fakeRootScope.$broadcast,
......
...@@ -9,7 +9,6 @@ const mockImportedComponents = require('./mock-imported-components'); ...@@ -9,7 +9,6 @@ const mockImportedComponents = require('./mock-imported-components');
describe('SelectionTabs', function() { describe('SelectionTabs', function() {
// mock services // mock services
let fakeSession;
let fakeSettings; let fakeSettings;
let fakeStore; let fakeStore;
...@@ -20,23 +19,11 @@ describe('SelectionTabs', function() { ...@@ -20,23 +19,11 @@ describe('SelectionTabs', function() {
function createComponent(props) { function createComponent(props) {
return mount( return mount(
<SelectionTabs <SelectionTabs settings={fakeSettings} {...defaultProps} {...props} />
session={fakeSession}
settings={fakeSettings}
{...defaultProps}
{...props}
/>
); );
} }
beforeEach(() => { beforeEach(() => {
fakeSession = {
state: {
preferences: {
show_sidebar_tutorial: false,
},
},
};
fakeSettings = { fakeSettings = {
enableExperimentalNewNoteButton: false, enableExperimentalNewNoteButton: false,
}; };
...@@ -65,7 +52,7 @@ describe('SelectionTabs', function() { ...@@ -65,7 +52,7 @@ describe('SelectionTabs', function() {
}); });
const unavailableMessage = wrapper => const unavailableMessage = wrapper =>
wrapper.find('.annotation-unavailable-message__label').text(); wrapper.find('.selection-tabs__message').text();
context('displays selection tabs and counts', function() { context('displays selection tabs and counts', function() {
it('should display the tabs and counts of annotations and notes', function() { it('should display the tabs and counts of annotations and notes', function() {
...@@ -172,7 +159,7 @@ describe('SelectionTabs', function() { ...@@ -172,7 +159,7 @@ describe('SelectionTabs', function() {
const wrapper = createComponent({ const wrapper = createComponent({
isLoading: true, isLoading: true,
}); });
assert.isFalse(wrapper.exists('.annotation-unavailable-message__label')); assert.isFalse(wrapper.exists('.selection-tabs__message'));
}); });
it('should not display the longer version of the no annotations message when there are no annotations and isWaitingToAnchorAnnotations is true', function() { it('should not display the longer version of the no annotations message when there are no annotations and isWaitingToAnchorAnnotations is true', function() {
...@@ -181,7 +168,7 @@ describe('SelectionTabs', function() { ...@@ -181,7 +168,7 @@ describe('SelectionTabs', function() {
const wrapper = createComponent({ const wrapper = createComponent({
isLoading: false, isLoading: false,
}); });
assert.isFalse(wrapper.exists('.annotation-unavailable-message__label')); assert.isFalse(wrapper.exists('.selection-tabs__message'));
}); });
it('should display the longer version of the no notes message when there are no notes', function() { it('should display the longer version of the no notes message when there are no notes', function() {
...@@ -196,24 +183,6 @@ describe('SelectionTabs', function() { ...@@ -196,24 +183,6 @@ describe('SelectionTabs', function() {
); );
}); });
it('should display the prompt to create a note when there are no notes and enableExperimentalNewNoteButton is true', function() {
fakeSettings.enableExperimentalNewNoteButton = true;
fakeStore.getState.returns({
selection: { selectedTab: uiConstants.TAB_NOTES },
});
fakeStore.noteCount.returns(0);
const wrapper = createComponent({});
assert.include(
wrapper.find('.annotation-unavailable-message__tutorial').text(),
'Create one by clicking the'
);
assert.isTrue(
wrapper
.find('.annotation-unavailable-message__tutorial i')
.hasClass('h-icon-note')
);
});
it('should display the longer version of the no annotations message when there are no annotations', function() { it('should display the longer version of the no annotations message when there are no annotations', function() {
fakeStore.annotationCount.returns(0); fakeStore.annotationCount.returns(0);
const wrapper = createComponent({}); const wrapper = createComponent({});
...@@ -222,49 +191,9 @@ describe('SelectionTabs', function() { ...@@ -222,49 +191,9 @@ describe('SelectionTabs', function() {
'There are no annotations in this group.' 'There are no annotations in this group.'
); );
assert.include( assert.include(
wrapper.find('.annotation-unavailable-message__tutorial').text(), unavailableMessage(wrapper),
'Create one by selecting some text and clicking the' 'Create one by selecting some text and clicking the'
); );
assert.isTrue(
wrapper
.find('.annotation-unavailable-message__tutorial i')
.hasClass('h-icon-annotate')
);
});
context('when the sidebar tutorial is displayed', function() {
it('should display the shorter version of the no notes message when there are no notes', function() {
fakeSession.state.preferences.show_sidebar_tutorial = true;
fakeStore.getState.returns({
selection: { selectedTab: uiConstants.TAB_NOTES },
});
fakeStore.noteCount.returns(0);
const wrapper = createComponent({});
const msg = unavailableMessage(wrapper);
assert.include(msg, 'There are no page notes in this group.');
assert.notInclude(msg, 'Create one by clicking the');
assert.notInclude(
msg,
'Create one by selecting some text and clicking the'
);
});
it('should display the shorter version of the no annotations message when there are no annotations', function() {
fakeSession.state.preferences.show_sidebar_tutorial = true;
fakeStore.annotationCount.returns(0);
const wrapper = createComponent({});
const msg = unavailableMessage(wrapper);
assert.include(msg, 'There are no annotations in this group.');
assert.notInclude(msg, 'Create one by clicking the');
assert.notInclude(
msg,
'Create one by selecting some text and clicking the'
);
});
}); });
}); });
}); });
@use "../../variables" as var; @use "../../variables" as var;
.new-note__create { .new-note-button {
background-color: var.$grey-mid;
border: none;
border-radius: 3px;
color: #fff;
display: flex; display: flex;
font-weight: 500; justify-content: flex-end;
margin-left: auto; margin: 0 1em 1em 0;
margin-right: 14px;
margin-bottom: 10px;
text-align: center;
} }
...@@ -48,3 +48,17 @@ ...@@ -48,3 +48,17 @@
.selection-tabs__type--orphan { .selection-tabs__type--orphan {
margin-left: -5px; margin-left: -5px;
} }
.selection-tabs__message {
border: 1px solid var.$grey-3;
padding: 3em;
text-align: center;
}
.selection-tabs__icon {
width: 12px;
height: 12px;
margin-right: 1px;
margin-bottom: -1px; // Pull the icon a little toward the baseline
color: var.$grey-5;
}
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