Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(drawing): add color picker component #1300

Merged
merged 32 commits into from
Dec 3, 2020
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
8a7294c
feat(drawing): add color picker component
ChenCodes Dec 1, 2020
dd420a0
chore(drawing): remove magic number
ChenCodes Dec 2, 2020
7001a2a
chore(drawing): use sibling selector
ChenCodes Dec 2, 2020
0c80c2a
chore(drawing): group imports / remove log
ChenCodes Dec 2, 2020
b231a3f
chore(drawing): flatten styles / do not style ControlsBar
ChenCodes Dec 2, 2020
817eb1d
chore(drawing): move ColorPickerPalette styles to its designated file
ChenCodes Dec 2, 2020
16657da
chore(drawing): fix eslint issues
ChenCodes Dec 2, 2020
0471d7f
chore(drawing): fix remaining lint issues
ChenCodes Dec 2, 2020
83187a6
chore(drawing): clean up styles / unused imports
ChenCodes Dec 2, 2020
68394ec
chore(drawing): add ColorPickerPalette test
ChenCodes Dec 3, 2020
fdf5cff
chore(drawing): add ColorPickerToggle test
ChenCodes Dec 3, 2020
011df39
chore(drawing): use hasClass
ChenCodes Dec 3, 2020
efc5e41
chore(drawing): rename as ColorPickerControl
ChenCodes Dec 3, 2020
589d2d2
chore(drawing): simplify sibling selector
ChenCodes Dec 3, 2020
731eec9
chore(drawing): pass color up through tree
ChenCodes Dec 3, 2020
2c90cdd
chore(drawing): add jest fn to test
ChenCodes Dec 3, 2020
c9ed901
chore(drawing): update DocBaseViewer test
ChenCodes Dec 3, 2020
9799954
chore(drawing): remove ControlsBar from ColorPickerPalette
ChenCodes Dec 3, 2020
4084bc5
chore(drawing): remove the explicit height
ChenCodes Dec 3, 2020
33fd6ee
chore(drawing): add shared variable
ChenCodes Dec 3, 2020
333800d
chore(drawing): update ColorPickerControl test
ChenCodes Dec 3, 2020
05763d8
chore(drawing): use border instead of box shadow
ChenCodes Dec 3, 2020
b64b1a7
chore(drawing): style the class instead of element
ChenCodes Dec 3, 2020
8d549cb
chore(drawing): revert back to using box shadow
ChenCodes Dec 3, 2020
261831e
chore(drawing): revert controls changes
ChenCodes Dec 3, 2020
53afaf1
chore(drawing): define bp-controls-background in styles file
ChenCodes Dec 3, 2020
8e5cedb
chore(drawing): refactor control / palette styles
ChenCodes Dec 3, 2020
a2f2636
chore(drawing): remove unused properties
ChenCodes Dec 3, 2020
c487c4d
chore(drawing): use #fff instead of white
ChenCodes Dec 3, 2020
1e4b91b
chore(drawing): set width and height as 18px instead of 16px
ChenCodes Dec 3, 2020
dcb5829
chore(drawing): remove unused import
ChenCodes Dec 3, 2020
e6017f1
Merge branch 'master' into add-color-picker-component
ChenCodes Dec 3, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/lib/viewers/controls/_styles.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
@import '~box-ui-elements/es/styles/variables';

$bp-controls-background: fade-out($black, .2);

@mixin bp-ControlButton($height: 48px, $width: 48px) {
display: flex;
align-items: center;
Expand Down
42 changes: 42 additions & 0 deletions src/lib/viewers/controls/color-picker/ColorPickerControl.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
@import '../styles';

.bp-ColorPickerControl {
position: relative;
display: flex;
align-items: center;
justify-content: center;
}

.bp-ColorPickerControl-button {
@include bp-ControlButton;
}

.bp-ColorPickerControl-palette {
$arrow-height: 12px;

position: absolute;
top: -#{$arrow-height};
left: 50%;
z-index: 1;
padding-bottom: 10px;
transform: translate(-50%, -100%);

&::after {
position: absolute;
left: 50%;
display: block;
border-top: $arrow-height solid $bp-controls-background;
border-right: $arrow-height solid transparent;
border-left: $arrow-height solid transparent;
transform: translateX(-50%);
content: '';
}
}

.bp-ColorPickerControl-swatch {
width: 18px;
height: 18px;
background-color: $box-blue;
border: 2px solid #fff;
border-radius: 2px;
}
49 changes: 49 additions & 0 deletions src/lib/viewers/controls/color-picker/ColorPickerControl.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import React, { useState } from 'react';
import classNames from 'classnames';
import ColorPickerPalette from './ColorPickerPalette';
import { AnnotationMode } from '../annotations/types';
import './ColorPickerControl.scss';

export type Props = {
annotationMode?: AnnotationMode;
onAnnotationColorClick: (color: string) => void;
isActive?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: 🔤

};

export default function ColorPickerControl({
annotationMode,
isActive = false,
onAnnotationColorClick,
...rest
}: Props): JSX.Element | null {
const [isColorPickerToggled, setIsColorPickerToggled] = useState(false);

if (annotationMode !== AnnotationMode.DRAWING) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this picker need to know about annotations at all? Could the rendering of this ColorPickerControl be controlled by the consuming component?

return null;
}

return (
<div className="bp-ColorPickerControl">
{isColorPickerToggled && (
<div className="bp-ColorPickerControl-palette">
<ColorPickerPalette
onColorSelect={(color: string): void => {
setIsColorPickerToggled(false);
onAnnotationColorClick(color);
}}
/>
</div>
)}
<button
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need a resin tag and/or testid attributes? Similarly for the ColorPickerPalette

className={classNames('bp-ColorPickerControl-button', {
'bp-is-active': isActive,
})}
onClick={(): void => setIsColorPickerToggled(!isColorPickerToggled)}
type="button"
{...rest}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a bit odd to spread the rest props onto a nested elements instead of the root element. Is this needed for something?

>
<div className="bp-ColorPickerControl-swatch" />
</button>
</div>
);
}
26 changes: 26 additions & 0 deletions src/lib/viewers/controls/color-picker/ColorPickerPalette.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
@import '../styles';

.bp-ColorPickerPalette {
display: flex;
align-items: center;
justify-content: center;
padding: 0 8px;
background: $bp-controls-background;
border-radius: 4px;
}

.bp-ColorPickerPalette-button {
width: 20px;
height: 20px;
margin: 10px 2px;
padding: 0;
border: none;
border-radius: 4px;
cursor: pointer;

&:focus,
&:hover {
ChenCodes marked this conversation as resolved.
Show resolved Hide resolved
outline: 0;
box-shadow: 0 0 0 2px #fff;
}
}
28 changes: 28 additions & 0 deletions src/lib/viewers/controls/color-picker/ColorPickerPalette.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import React from 'react';
import './ColorPickerPalette.scss';

export type Props = {
onColorSelect: (color: string) => void;
};

export default function ColorPickerPalette({ onColorSelect }: Props): JSX.Element {
const colors = ['#0061d5', '#26c281', '#ed3757', '#f5b31b', '#ffd700', '#4826c2'];
ChenCodes marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these colors be moved outside of the function?


return (
<div className="bp-ColorPickerPalette">
{colors.map(color => {
return (
<button
key={color}
className="bp-ColorPickerPalette-button"
onClick={(): void => onColorSelect(color)}
style={{
backgroundColor: color,
}}
type="button"
/>
);
})}
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import * as React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import { AnnotationMode } from '../../annotations/types';
import ColorPickerControl from '../ColorPickerControl';

describe('ColorPickerControl', () => {
const onAnnotationColorClick = jest.fn();

const getWrapper = (props = {}): ShallowWrapper =>
shallow(<ColorPickerControl onAnnotationColorClick={onAnnotationColorClick} {...props} />);

const getToggleButton = (wrapper: ShallowWrapper): ShallowWrapper => wrapper.find('.bp-ColorPickerControl-button');

describe('render', () => {
test('should render null if annotationMode is not AnnotationMode.DRAWING', () => {
const wrapper = getWrapper({ annotationMode: AnnotationMode.REGION });

expect(wrapper.isEmptyRender()).toBe(true);
});

test('should not render ColorPickerPalette when the component is first mounted', () => {
const wrapper = getWrapper();

expect(wrapper.find('ColorPickerPalette').exists()).toBe(false);
});

test('should render ColorPickerPalette when the toggle button is clicked', () => {
const wrapper = getWrapper({ annotationMode: AnnotationMode.DRAWING });

getToggleButton(wrapper).simulate('click');

expect(wrapper.find('ColorPickerPalette').exists()).toBe(true);
});

test('should render the toggle button with bp-is-active set to true if isActive is true', () => {
const wrapper = getWrapper({ annotationMode: AnnotationMode.DRAWING, isActive: true });

expect(getToggleButton(wrapper).hasClass('bp-is-active')).toBe(true);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import ColorPickerPalette from '../ColorPickerPalette';

describe('ColorPickerPalette', () => {
const onColorSelect = jest.fn();

const getWrapper = (props = {}): ShallowWrapper =>
shallow(<ColorPickerPalette onColorSelect={onColorSelect} {...props} />);

describe('render', () => {
test('should render the six color swatches', () => {
const wrapper = getWrapper();

expect(wrapper.find('button').length).toBe(6);
});
});
});
2 changes: 2 additions & 0 deletions src/lib/viewers/controls/color-picker/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './ColorPickerControl';
export { default } from './ColorPickerControl';
4 changes: 4 additions & 0 deletions src/lib/viewers/controls/controls-bar/ControlsBar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@
align-items: center;
background: fade-out($black, .2);
border-radius: 3px;

& + & {
margin-left: 6px;
}
}
6 changes: 6 additions & 0 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ class DocBaseViewer extends BaseViewer {
this.emitMetric = this.emitMetric.bind(this);
this.handleAssetAndRepLoad = this.handleAssetAndRepLoad.bind(this);
this.handleFindBarClose = this.handleFindBarClose.bind(this);
this.handleAnnotationColorClick = this.handleAnnotationColorClick.bind(this);
this.handleAnnotationControlsClick = this.handleAnnotationControlsClick.bind(this);
this.handleAnnotationControlsEscape = this.handleAnnotationControlsEscape.bind(this);
this.handleAnnotationCreateEvent = this.handleAnnotationCreateEvent.bind(this);
Expand Down Expand Up @@ -1094,6 +1095,7 @@ class DocBaseViewer extends BaseViewer {
hasRegion={canAnnotate}
maxScale={MAX_SCALE}
minScale={MIN_SCALE}
onAnnotationColorClick={this.handleAnnotationColorClick}
onAnnotationModeClick={this.handleAnnotationControlsClick}
onAnnotationModeEscape={this.handleAnnotationControlsEscape}
onFindBarToggle={this.toggleFindBar}
Expand Down Expand Up @@ -1669,6 +1671,10 @@ class DocBaseViewer extends BaseViewer {
}
}

handleAnnotationColorClick() {
// TODO: Will implement in a separate PR
}

handleAnnotationControlsClick({ mode }) {
const nextMode = this.annotationControlsFSM.transition(AnnotationInput.CLICK, mode);
this.annotator.toggleAnnotationMode(
Expand Down
60 changes: 34 additions & 26 deletions src/lib/viewers/doc/DocControls.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import AnnotationsControls, { Props as AnnotationsControlsProps } from '../controls/annotations';
import ColorPickerControl, { Props as ColorPickerControlProps } from '../controls/color-picker';
import ControlsBar from '../controls/controls-bar';
import FindBarToggle, { Props as FindBarToggleProps } from '../controls/findbar';
import FullscreenToggle, { Props as FullscreenToggleProps } from '../controls/fullscreen';
Expand All @@ -8,6 +9,7 @@ import ThumbnailsToggle, { Props as ThumbnailsToggleProps } from '../controls/si
import ZoomControls, { Props as ZoomControlsProps } from '../controls/zoom';

export type Props = AnnotationsControlsProps &
ColorPickerControlProps &
FindBarToggleProps &
FullscreenToggleProps &
PageControlsProps &
Expand All @@ -21,6 +23,7 @@ export default function DocControls({
hasRegion,
maxScale,
minScale,
onAnnotationColorClick,
onAnnotationModeClick,
onAnnotationModeEscape,
onFindBarToggle,
Expand All @@ -35,31 +38,36 @@ export default function DocControls({
scale,
}: Props): JSX.Element {
return (
<ControlsBar>
<ThumbnailsToggle onThumbnailsToggle={onThumbnailsToggle} />
<FindBarToggle onFindBarToggle={onFindBarToggle} />
<ZoomControls
maxScale={maxScale}
minScale={minScale}
onZoomIn={onZoomIn}
onZoomOut={onZoomOut}
scale={scale}
/>
<PageControls
onPageChange={onPageChange}
onPageSubmit={onPageSubmit}
pageCount={pageCount}
pageNumber={pageNumber}
/>
<FullscreenToggle onFullscreenToggle={onFullscreenToggle} />
<AnnotationsControls
annotationMode={annotationMode}
hasDrawing={hasDrawing}
hasHighlight={hasHighlight}
hasRegion={hasRegion}
onAnnotationModeClick={onAnnotationModeClick}
onAnnotationModeEscape={onAnnotationModeEscape}
/>
</ControlsBar>
<>
<ControlsBar>
<ThumbnailsToggle onThumbnailsToggle={onThumbnailsToggle} />
<FindBarToggle onFindBarToggle={onFindBarToggle} />
<ZoomControls
maxScale={maxScale}
minScale={minScale}
onZoomIn={onZoomIn}
onZoomOut={onZoomOut}
scale={scale}
/>
<PageControls
onPageChange={onPageChange}
onPageSubmit={onPageSubmit}
pageCount={pageCount}
pageNumber={pageNumber}
/>
<FullscreenToggle onFullscreenToggle={onFullscreenToggle} />
<AnnotationsControls
annotationMode={annotationMode}
hasDrawing={hasDrawing}
hasHighlight={hasHighlight}
hasRegion={hasRegion}
onAnnotationModeClick={onAnnotationModeClick}
onAnnotationModeEscape={onAnnotationModeEscape}
/>
</ControlsBar>
<ControlsBar>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be added to the ImageControls as well?

<ColorPickerControl annotationMode={annotationMode} onAnnotationColorClick={onAnnotationColorClick} />
</ControlsBar>
</>
);
}
1 change: 1 addition & 0 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1725,6 +1725,7 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
hasRegion={false}
maxScale={10}
minScale={0.1}
onAnnotationColorClick={docBase.handleAnnotationColorClick}
onAnnotationModeClick={docBase.handleAnnotationControlsClick}
onAnnotationModeEscape={docBase.handleAnnotationControlsEscape}
onFindBarToggle={docBase.toggleFindBar}
Expand Down