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

refactor(controls): Adjusting the size of the grouped controls #1096

Merged
merged 4 commits into from
Nov 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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);
jstoffan marked this conversation as resolved.
Show resolved Hide resolved

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}`;
ConradJChan marked this conversation as resolved.
Show resolved Hide resolved

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;
ConradJChan marked this conversation as resolved.
Show resolved Hide resolved
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.