Skip to content

Commit

Permalink
refactor(controls): Adjusting the size of the grouped controls (#1096)
Browse files Browse the repository at this point in the history
* refactor(controls): Adjusting the size of the grouped controls

* chore: fix unit tests

* chore: PR comments and make svgs not focusable
  • Loading branch information
ConradJChan authored and mergify[bot] committed Nov 7, 2019
1 parent fcae150 commit b04e7e6
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 22 deletions.
25 changes: 23 additions & 2 deletions src/lib/Controls.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ const CONTROLS_PAGE_NUM_INPUT_CLASS = 'bp-page-num-input';
const CONTROLS_PAGE_NUM_WRAPPER_CLASS = 'bp-page-num-wrapper';
const CONTROLS_AUTO_HIDE_TIMEOUT_IN_MILLIS = 2000;

export const CLASS_BOX_CONTROLS_GROUP_BUTTON = 'bp-controls-group-btn';

class Controls {
/** @property {HTMLElement} - Controls container element */
containerEl;
Expand Down Expand Up @@ -191,9 +193,12 @@ class Controls {
* @param {string} [classList] - optional class list
* @param {string} [content] - Optional content HTML
* @param {string} [tag] - Optional html tag, defaults to 'button'
* @param {HTMLElement} [parent] - Optional parent tag, defaults to the controls element
* @return {HTMLElement} The created HTMLElement inserted into the control
*/
add(text, handler, classList = '', content = '', tag = 'button') {
add(text, handler, classList = '', content = '', tag = 'button', parent = this.controlsEl) {
const parentElement = this.controlsEl.contains(parent) ? parent : this.controlsEl;

const cell = document.createElement('div');
cell.className = 'bp-controls-cell';

Expand All @@ -214,7 +219,7 @@ class Controls {
}

cell.appendChild(element);
this.controlsEl.appendChild(cell);
parentElement.appendChild(cell);

if (handler) {
// Maintain a reference for cleanup
Expand All @@ -227,6 +232,22 @@ class Controls {
return element;
}

/**
* Add div for a group of controls
*
* @public
* @param {string} classNames - optional class names
* @return {HTMLElement} The created HTMLElement for a group of controls
*/
addGroup(classNames = '') {
const group = document.createElement('div');
group.className = `bp-controls-group ${classNames}`;

this.controlsEl.appendChild(group);

return group;
}

/**
* Enables the controls.
*
Expand Down
26 changes: 24 additions & 2 deletions src/lib/Controls.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
.bp-controls {
position: relative;
left: -50%;
display: table;
table-layout: fixed;
display: flex;
background: fade-out($twos, .05);
border-radius: 3px;
opacity: 0;
Expand Down Expand Up @@ -81,6 +80,12 @@
visibility: visible;
}
}

.bp-zoom-btn {
display: flex;
align-items: center;
justify-content: center;
}
}

.box-show-preview-controls .bp-controls {
Expand Down Expand Up @@ -155,6 +160,23 @@
}

.bp-zoom-current-scale {
min-width: 48px;
color: $white;
font-size: 14px;
text-align: center;
}

.bp-controls-group {
display: flex;
align-items: center;
margin-right: 4px;
margin-left: 4px;

.bp-controls-group-btn {
width: 32px;
}

& + .bp-controls-cell {
margin-left: 4px;
}
}
25 changes: 22 additions & 3 deletions src/lib/PageControls.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Browser from './Browser';
import { BROWSERS } from './constants';
import { decodeKeydown } from './util';
import { ICON_DROP_DOWN, ICON_DROP_UP } from './icons/icons';
import { CLASS_BOX_CONTROLS_GROUP_BUTTON } from './Controls';

const SHOW_PAGE_NUM_INPUT_CLASS = 'show-page-number-input';
const CONTROLS_PAGE_NUM_WRAPPER_CLASS = 'bp-page-num-wrapper';
Expand Down Expand Up @@ -72,14 +73,32 @@ class PageControls extends EventEmitter {
* @return {void}
*/
add(currentPageNumber, pagesCount) {
const groupElement = this.controls.addGroup();
// const groupElement = undefined;
this.controls.add(
__('previous_page'),
this.setPreviousPage,
`bp-previous-page-icon ${PREV_PAGE}`,
`${CLASS_BOX_CONTROLS_GROUP_BUTTON} bp-previous-page-icon ${PREV_PAGE}`,
ICON_DROP_UP,
undefined,
groupElement,
);
this.controls.add(
__('enter_page_num'),
this.showPageNumInput,
PAGE_NUM,
pageNumTemplate,
undefined,
groupElement,
);
this.controls.add(
__('next_page'),
this.setNextPage,
`${CLASS_BOX_CONTROLS_GROUP_BUTTON} bp-next-page-icon ${NEXT_PAGE}`,
ICON_DROP_DOWN,
undefined,
groupElement,
);
this.controls.add(__('enter_page_num'), this.showPageNumInput, PAGE_NUM, pageNumTemplate);
this.controls.add(__('next_page'), this.setNextPage, `bp-next-page-icon ${NEXT_PAGE}`, ICON_DROP_DOWN);

const pageNumEl = this.controlsEl.querySelector(`.${PAGE_NUM}`);
this.totalPagesEl = pageNumEl.querySelector(`.${CONTROLS_TOTAL_PAGES}`);
Expand Down
27 changes: 22 additions & 5 deletions src/lib/ZoomControls.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import isFinite from 'lodash/isFinite';
import noop from 'lodash/noop';
import { ICON_ZOOM_IN, ICON_ZOOM_OUT } from './icons/icons';
import Controls from './Controls';
import Controls, { CLASS_BOX_CONTROLS_GROUP_BUTTON } from './Controls';

const CLASS_ZOOM_CURRENT_SCALE = 'bp-zoom-current-scale';
const CLASS_ZOOM_IN_BUTTON = 'bp-zoom-in-btn';
const CLASS_ZOOM_OUT_BUTTON = 'bp-zoom-out-btn';
const CLASS_ZOOM_BUTTON = 'bp-zoom-btn';

class ZoomControls {
/** @property {Controls} - Controls object */
Expand Down Expand Up @@ -67,15 +68,31 @@ class ZoomControls {
this.maxZoom = Math.round(this.validateZoom(maxZoom, Number.POSITIVE_INFINITY) * 100);
this.minZoom = Math.round(Math.max(this.validateZoom(minZoom, 0), 0) * 100);

this.controls.add(__('zoom_out'), onZoomOut, `${CLASS_ZOOM_OUT_BUTTON} ${zoomOutClassName}`, ICON_ZOOM_OUT);
const groupElement = this.controls.addGroup();
this.controls.add(
__('zoom_current_scale'),
__('zoom_out'),
onZoomOut,
`${CLASS_BOX_CONTROLS_GROUP_BUTTON} ${CLASS_ZOOM_BUTTON} ${CLASS_ZOOM_OUT_BUTTON} ${zoomOutClassName}`,
ICON_ZOOM_OUT,
undefined,
groupElement,
);
this.controls.add(
__('zoom_current_scale'),
undefined,
`<span class="${CLASS_ZOOM_CURRENT_SCALE}" data-testid="current-zoom">100%</span>`,
CLASS_ZOOM_CURRENT_SCALE,
'<span data-testid="current-zoom">100%</span>',
'div',
groupElement,
);
this.controls.add(
__('zoom_in'),
onZoomIn,
`${CLASS_BOX_CONTROLS_GROUP_BUTTON} ${CLASS_ZOOM_BUTTON} ${CLASS_ZOOM_IN_BUTTON} ${zoomInClassName}`,
ICON_ZOOM_IN,
undefined,
groupElement,
);
this.controls.add(__('zoom_in'), onZoomIn, `${CLASS_ZOOM_IN_BUTTON} ${zoomInClassName}`, ICON_ZOOM_IN);

this.currentScaleElement = this.controlsElement.querySelector(`.${CLASS_ZOOM_CURRENT_SCALE}`);
this.setCurrentScale(currentScale);
Expand Down
14 changes: 14 additions & 0 deletions src/lib/__tests__/Controls-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,20 @@ describe('lib/Controls', () => {
expect(span.parentNode.parentNode).to.equal(controls.controlsEl);
expect(controls.buttonRefs.push).not.to.be.called;
});

it('should append the controls to the provided element', () => {
const div = controls.addGroup('test-group');
const btn = controls.add('test button', sandbox.stub(), 'test1', 'test content', undefined, div);
expect(btn.parentNode.parentNode).to.equal(div);
});
});

describe('addGroup()', () => {
it('should create a controls group within the controls element', () => {
const div = controls.addGroup('test-group');
expect(div.parentNode).to.equal(controls.controlsEl);
expect(div.classList.contains('test-group'));
});
});

describe('enable()', () => {
Expand Down
32 changes: 26 additions & 6 deletions src/lib/__tests__/ZoomControls-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,30 @@ describe('lib/ZoomControls', () => {
it('should add the controls', () => {
zoomControls.init(0.5, { onZoomIn: stubs.onZoomIn, onZoomOut: stubs.onZoomOut });

expect(stubs.add).to.be.calledWith(__('zoom_out'), stubs.onZoomOut, 'bp-zoom-out-btn ', ICON_ZOOM_OUT);
expect(stubs.add).to.be.calledWith(
__('zoom_current_scale'),
__('zoom_out'),
stubs.onZoomOut,
'bp-controls-group-btn bp-zoom-btn bp-zoom-out-btn ',
ICON_ZOOM_OUT,
undefined,
sinon.match.any,
);
expect(stubs.add).to.be.calledWith(
__('zoom_current_scale'),
undefined,
'bp-zoom-current-scale',
sinon.match.string,
'div',
sinon.match.any,
);
expect(stubs.add).to.be.calledWith(
__('zoom_in'),
stubs.onZoomIn,
'bp-controls-group-btn bp-zoom-btn bp-zoom-in-btn ',
ICON_ZOOM_IN,
undefined,
sinon.match.any,
);
expect(stubs.add).to.be.calledWith(__('zoom_in'), stubs.onZoomIn, 'bp-zoom-in-btn ', ICON_ZOOM_IN);
expect(zoomControls.currentScaleElement).not.to.be.undefined;
expect(stubs.setCurrentScale).to.be.calledWith(0.5);
expect(zoomControls.maxZoom).to.be.equal(Number.POSITIVE_INFINITY);
Expand Down Expand Up @@ -102,21 +117,26 @@ describe('lib/ZoomControls', () => {
expect(stubs.add).to.be.calledWith(
__('zoom_out'),
stubs.onZoomOut,
'bp-zoom-out-btn zoom-out-classname',
'bp-controls-group-btn bp-zoom-btn bp-zoom-out-btn zoom-out-classname',
ICON_ZOOM_OUT,
undefined,
sinon.match.any,
);
expect(stubs.add).to.be.calledWith(
__('zoom_current_scale'),
undefined,
undefined,
'bp-zoom-current-scale',
sinon.match.string,
'div',
sinon.match.any,
);
expect(stubs.add).to.be.calledWith(
__('zoom_in'),
stubs.onZoomIn,
'bp-zoom-in-btn zoom-in-classname',
'bp-controls-group-btn bp-zoom-btn bp-zoom-in-btn zoom-in-classname',
ICON_ZOOM_IN,
undefined,
sinon.match.any,
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion src/lib/icons/search.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion src/lib/icons/thumbnails-toggle-icon.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion src/lib/icons/zoom_in.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion src/lib/icons/zoom_out.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit b04e7e6

Please sign in to comment.