-
Notifications
You must be signed in to change notification settings - Fork 116
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(controls): Add react versions of fullscreen and zoom controls #1283
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
@import '~box-ui-elements/es/styles/variables'; | ||
|
||
@mixin bp-ControlButton($height: 48px, $width: 48px) { | ||
display: flex; | ||
align-items: center; | ||
justify-content: center; | ||
width: $width; | ||
height: $height; | ||
color: $white; | ||
background: transparent; | ||
border: 1px solid transparent; | ||
outline: 0; | ||
cursor: pointer; | ||
opacity: .7; | ||
transition: opacity 150ms; | ||
user-select: none; | ||
touch-action: manipulation; | ||
zoom: 1; | ||
|
||
&:focus, | ||
&:hover { | ||
opacity: 1; | ||
} | ||
|
||
&:focus { | ||
box-shadow: inset 0 0 0 1px fade-out($white, .5), 0 1px 2px fade-out($black, .9); | ||
} | ||
|
||
&:disabled { | ||
cursor: default; | ||
opacity: .2; | ||
pointer-events: none; | ||
} | ||
} | ||
|
||
@mixin bp-ControlGroup { | ||
display: flex; | ||
align-items: center; | ||
margin-right: 4px; | ||
margin-left: 4px; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
@import 'src/lib/viewers/controls/styles'; | ||
|
||
.bp-FullscreenToggle { | ||
@include bp-ControlButton; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import React from 'react'; | ||
import fullscreen from '../../../Fullscreen'; | ||
import IconFullscreenIn24 from '../icons/IconFullscreenIn24'; | ||
import IconFullscreenOut24 from '../icons/IconFullscreenOut24'; | ||
import './FullscreenToggle.scss'; | ||
|
||
export type Props = { | ||
onFullscreenToggle: (isFullscreen: boolean) => void; | ||
}; | ||
|
||
export default function FullscreenToggle({ onFullscreenToggle }: Props): JSX.Element { | ||
const [isFullscreen, setFullscreen] = React.useState(false); | ||
const Icon = isFullscreen ? IconFullscreenOut24 : IconFullscreenIn24; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these be reversed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so? |
||
const title = isFullscreen ? __('exit_fullscreen') : __('enter_fullscreen'); | ||
|
||
const handleClick = (): void => { | ||
onFullscreenToggle(!isFullscreen); | ||
}; | ||
|
||
React.useEffect(() => { | ||
const handleFullscreenEnter = (): void => setFullscreen(true); | ||
const handleFullscreenExit = (): void => setFullscreen(false); | ||
|
||
fullscreen.addListener('enter', handleFullscreenEnter); | ||
fullscreen.addListener('exit', handleFullscreenExit); | ||
|
||
return (): void => { | ||
fullscreen.removeListener('enter', handleFullscreenEnter); | ||
fullscreen.removeListener('exit', handleFullscreenExit); | ||
}; | ||
}, []); | ||
|
||
return ( | ||
<button className="bp-FullscreenToggle" onClick={handleClick} title={title} type="button"> | ||
<Icon /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we introduce Tooltip here in these buttons or will that be a follow on enhancement? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is meant to replicate the existing experience, so we're still using |
||
</button> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import React from 'react'; | ||
import { shallow, ShallowWrapper } from 'enzyme'; | ||
import fullscreen from '../../../../Fullscreen'; | ||
import FullscreenToggle from '../FullscreenToggle'; | ||
import IconFullscreenIn24 from '../../icons/IconFullscreenIn24'; | ||
import IconFullscreenOut24 from '../../icons/IconFullscreenOut24'; | ||
|
||
describe('FullscreenToggle', () => { | ||
const getWrapper = (props = {}): ShallowWrapper => | ||
shallow(<FullscreenToggle onFullscreenToggle={jest.fn()} {...props} />); | ||
|
||
beforeEach(() => { | ||
jest.spyOn(React, 'useEffect').mockImplementation(fn => fn()); | ||
}); | ||
|
||
describe('event handlers', () => { | ||
test('should respond to fullscreen events', () => { | ||
const wrapper = getWrapper(); | ||
|
||
fullscreen.enter(); | ||
expect(wrapper.exists(IconFullscreenOut24)).toBe(true); | ||
expect(wrapper.prop('title')).toBe(__('exit_fullscreen')); | ||
|
||
fullscreen.exit(); | ||
expect(wrapper.exists(IconFullscreenIn24)).toBe(true); | ||
expect(wrapper.prop('title')).toBe(__('enter_fullscreen')); | ||
}); | ||
|
||
test('should invoke onFullscreenToggle prop on click', () => { | ||
const onToggle = jest.fn(); | ||
const wrapper = getWrapper({ onFullscreenToggle: onToggle }); | ||
|
||
wrapper.simulate('click'); | ||
expect(onToggle).toBeCalledWith(true); | ||
}); | ||
}); | ||
|
||
describe('render', () => { | ||
test('should return a valid wrapper', () => { | ||
const wrapper = getWrapper(); | ||
|
||
expect(wrapper.hasClass('bp-FullscreenToggle')).toBe(true); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from './FullscreenToggle'; | ||
export { default } from './FullscreenToggle'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,11 @@ | ||
import * as React from 'react'; | ||
|
||
function IconZoomOut(props: React.SVGProps<SVGSVGElement>): JSX.Element { | ||
function IconZoomOut10(props: React.SVGProps<SVGSVGElement>): JSX.Element { | ||
return ( | ||
<svg focusable="false" height={10} viewBox="0 0 10 10" width={10} {...props}> | ||
<rect fill="#FFF" fillRule="evenodd" height={2} rx={1} width={10} y={5} /> | ||
</svg> | ||
); | ||
} | ||
|
||
export default IconZoomOut; | ||
export default IconZoomOut10; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
@import '../styles'; | ||
|
||
.bp-ZoomControls { | ||
@include bp-ControlGroup; | ||
} | ||
|
||
.bp-ZoomControls-button { | ||
@include bp-ControlButton($width: 32px); | ||
} | ||
|
||
.bp-ZoomControls-current { | ||
display: flex; | ||
align-items: center; | ||
justify-content: center; | ||
min-width: 48px; | ||
color: #fff; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we use the SCSS variable from box-ui-elements? |
||
font-size: 14px; | ||
user-select: none; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import React from 'react'; | ||
import isFinite from 'lodash/isFinite'; | ||
import IconZoomIn10 from '../icons/IconZoomIn10'; | ||
import IconZoomOut10 from '../icons/IconZoomOut10'; | ||
import './ZoomControls.scss'; | ||
|
||
export type Props = { | ||
maxScale?: number; | ||
minScale?: number; | ||
onZoomIn: () => void; | ||
onZoomOut: () => void; | ||
scale?: number; | ||
}; | ||
|
||
export const MAX_SCALE = 100; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so? A scale of |
||
export const MIN_SCALE = 0.1; | ||
|
||
export default function ZoomControls({ | ||
maxScale = MAX_SCALE, | ||
minScale = MIN_SCALE, | ||
onZoomIn, | ||
onZoomOut, | ||
scale = 1, | ||
}: Props): JSX.Element { | ||
const currentZoom = Math.round(scale * 100); | ||
const maxScaleValue = isFinite(maxScale) ? Math.min(maxScale, MAX_SCALE) : MAX_SCALE; | ||
const minScaleValue = isFinite(minScale) ? Math.max(minScale, MIN_SCALE) : MIN_SCALE; | ||
|
||
return ( | ||
<div className="bp-ZoomControls"> | ||
<button | ||
className="bp-ZoomControls-button" | ||
data-testid="bp-ZoomControls-out" | ||
disabled={scale <= minScaleValue} | ||
onClick={onZoomOut} | ||
title={__('zoom_out')} | ||
type="button" | ||
> | ||
<IconZoomOut10 /> | ||
</button> | ||
<div | ||
className="bp-ZoomControls-current" | ||
data-testid="current-zoom" | ||
title={__('zoom_current_scale')} | ||
>{`${currentZoom}%`}</div> | ||
<button | ||
className="bp-ZoomControls-button" | ||
data-testid="bp-ZoomControls-in" | ||
disabled={scale >= maxScaleValue} | ||
onClick={onZoomIn} | ||
title={__('zoom_in')} | ||
type="button" | ||
> | ||
<IconZoomIn10 /> | ||
</button> | ||
</div> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
import React from 'react'; | ||
import noop from 'lodash/noop'; | ||
import { shallow, ShallowWrapper } from 'enzyme'; | ||
import ZoomControls from '../ZoomControls'; | ||
|
||
describe('ZoomControls', () => { | ||
const getWrapper = (props = {}): ShallowWrapper => | ||
shallow(<ZoomControls onZoomIn={noop} onZoomOut={noop} {...props} />); | ||
const getZoom = (wrapper: ShallowWrapper): ShallowWrapper => wrapper.find('[data-testid="current-zoom"]'); | ||
const getZoomIn = (wrapper: ShallowWrapper): ShallowWrapper => wrapper.find('[data-testid="bp-ZoomControls-in"]'); | ||
const getZoomOut = (wrapper: ShallowWrapper): ShallowWrapper => wrapper.find('[data-testid="bp-ZoomControls-out"]'); | ||
|
||
describe('event handlers', () => { | ||
test('should handle zoom in click', () => { | ||
const onZoomIn = jest.fn(); | ||
const wrapper = getWrapper({ onZoomIn }); | ||
|
||
getZoomIn(wrapper).simulate('click'); | ||
|
||
expect(onZoomIn).toBeCalled(); | ||
}); | ||
|
||
test('should handle zoom out click', () => { | ||
const onZoomOut = jest.fn(); | ||
const wrapper = getWrapper({ onZoomOut }); | ||
|
||
getZoomOut(wrapper).simulate('click'); | ||
|
||
expect(onZoomOut).toBeCalled(); | ||
}); | ||
}); | ||
|
||
describe('render', () => { | ||
test.each` | ||
minScale | scale | disabled | ||
${null} | ${1} | ${false} | ||
${0.5} | ${1} | ${false} | ||
${0.5} | ${0.5} | ${true} | ||
${-50} | ${0.1} | ${true} | ||
${-50} | ${0.2} | ${false} | ||
`('should set disabled for zoom out to $disabled for $scale / $minScale', ({ disabled, minScale, scale }) => { | ||
const wrapper = getWrapper({ minScale, scale }); | ||
|
||
expect(getZoomOut(wrapper).prop('disabled')).toBe(disabled); | ||
}); | ||
|
||
test.each` | ||
maxScale | scale | disabled | ||
${null} | ${1} | ${false} | ||
${10} | ${1} | ${false} | ||
${50} | ${10} | ${false} | ||
${50} | ${50} | ${true} | ||
${500} | ${100} | ${true} | ||
${500} | ${99} | ${false} | ||
`('should set disabled for zoom in to $disabled for $scale / $maxScale', ({ disabled, maxScale, scale }) => { | ||
const wrapper = getWrapper({ maxScale, scale }); | ||
|
||
expect(getZoomIn(wrapper).prop('disabled')).toBe(disabled); | ||
}); | ||
|
||
test.each` | ||
scale | zoom | ||
${1} | ${'100%'} | ||
${1.49} | ${'149%'} | ||
${1.499} | ${'150%'} | ||
${10} | ${'1000%'} | ||
${100} | ${'10000%'} | ||
`('should format $scale to $zoom properly', ({ scale, zoom }) => { | ||
const wrapper = getWrapper({ scale }); | ||
|
||
expect(getZoom(wrapper).text()).toEqual(zoom); | ||
}); | ||
|
||
test('should return a valid wrapper', () => { | ||
const wrapper = getWrapper(); | ||
|
||
expect(getZoom(wrapper)).toBeDefined(); | ||
expect(getZoomIn(wrapper)).toBeDefined(); | ||
expect(getZoomOut(wrapper)).toBeDefined(); | ||
expect(wrapper.hasClass('bp-ZoomControls')).toBe(true); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from './ZoomControls'; | ||
export { default } from './ZoomControls'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any benefit to have these icons in box-ui-elements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, since they're not part of our standardized icon set. We may refresh them at some point in the future, at which point that may make more sense.