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(docviewer): Toggle find bar control #1088

Merged
merged 2 commits into from
Nov 6, 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
2 changes: 2 additions & 0 deletions src/i18n/en-US.properties
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ loading_preview=Loading Preview...
download_file=Download File
# Text shown when a text file has been truncated due to size limits.
text_truncated=This file has been truncated due to size limits. Please download to view the whole file.
# Button tooltip for toggling find bar
toggle_findbar=Toggle findbar
# Button tooltip to toggle Thumbnails Sidebar
toggle_thumbnails=Toggle thumbnails
# Message when file has inaccessible xrefs
Expand Down
8 changes: 5 additions & 3 deletions src/lib/icons/search.svg
100755 → 100644
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 18 additions & 3 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
ICON_PRINT_CHECKMARK,
ICON_FULLSCREEN_IN,
ICON_FULLSCREEN_OUT,
ICON_SEARCH,
ICON_THUMBNAILS_TOGGLE,
} from '../../icons/icons';
import { JS, PRELOAD_JS, CSS } from './docAssets';
Expand Down Expand Up @@ -774,16 +775,26 @@ class DocBaseViewer extends BaseViewer {
// Only initialize the find bar if the user has download permissions on
// the file. Users without download permissions shouldn't be able to
// interact with the text layer
const canDownload = checkPermission(this.options.file, PERMISSION_DOWNLOAD);
if (!canDownload || this.getViewerOption('disableFindBar')) {
if (this.isFindDisabled()) {
return;
}

this.findBar = new DocFindBar(this.findBarEl, this.pdfFindController, this.pdfEventBus, canDownload);
this.findBar = new DocFindBar(this.findBarEl, this.pdfFindController, this.pdfEventBus);
this.findBar.addListener(VIEWER_EVENT.metric, this.emitMetric);
this.findBar.addListener('close', this.handleFindBarClose);
}

/**
* Determines if findbar is disabled
*
* @private
* @return {boolean}
*/
isFindDisabled() {
const canDownload = checkPermission(this.options.file, PERMISSION_DOWNLOAD);
Copy link
Contributor

Choose a reason for hiding this comment

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

very minor: I think the intention with this const is to be more readable so I would probably add the negation in the const creation and name it as such.

return !canDownload || this.getViewerOption('disableFindBar');
}

/**
* Re-sizing logic.
*
Expand Down Expand Up @@ -1069,6 +1080,10 @@ class DocBaseViewer extends BaseViewer {
);
}

if (!this.isFindDisabled()) {
this.controls.add(__('toggle_findbar'), () => this.findBar.toggle(), 'bp-toggle-findbar-icon', ICON_SEARCH);
}

this.zoomControls.init(this.pdfViewer.currentScale, {
maxZoom: MAX_SCALE,
minZoom: MIN_SCALE,
Expand Down
13 changes: 13 additions & 0 deletions src/lib/viewers/doc/DocFindBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,19 @@ class DocFindBar extends EventEmitter {
this.opened = false;
this.bar.classList.add(CLASS_HIDDEN);
}

/**
* Toggles the Find Bar open and closed
*
* @return {void}
*/
toggle() {
if (this.opened) {
this.close();
} else {
this.open();
}
}
}

export default DocFindBar;
13 changes: 8 additions & 5 deletions src/lib/viewers/doc/DocFindBar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,24 @@ $blue: #00f !default;
margin: 0;
line-height: 1;
text-align: left;
}
border: 0;

.bp-doc-find-close {
&:hover .icon {
fill: $better-black;
svg {
width: 12px;
height: 12px;
}
}

.bp-doc-find-search,
.bp-doc-find-close {
border: 0;

svg {
width: 22px;
}

&:hover .icon {
fill: $better-black;
}
}

.bp-find-match-not-found,
Expand Down
27 changes: 25 additions & 2 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ import {
SELECTOR_BOX_PREVIEW,
} from '../../../constants';
import {
ICON_PRINT_CHECKMARK,
ICON_THUMBNAILS_TOGGLE,
ICON_FULLSCREEN_IN,
ICON_FULLSCREEN_OUT,
ICON_PRINT_CHECKMARK,
ICON_SEARCH,
ICON_THUMBNAILS_TOGGLE,
} from '../../../icons/icons';
import { VIEWER_EVENT, LOAD_METRIC, USER_DOCUMENT_THUMBNAIL_EVENTS } from '../../../events';
import Timer from '../../../Timer';
Expand Down Expand Up @@ -2257,6 +2258,8 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
add: sandbox.stub(),
removeListener: sandbox.stub(),
};

stubs.isFindDisabled = sandbox.stub(docBase, 'isFindDisabled');
});

it('should add the correct controls', () => {
Expand All @@ -2269,6 +2272,13 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
ICON_THUMBNAILS_TOGGLE,
);

expect(docBase.controls.add).to.be.calledWith(
__('toggle_findbar'),
sinon.match.func,
'bp-toggle-findbar-icon',
ICON_SEARCH,
);

expect(docBase.zoomControls.init).to.be.calledWith(0.9, {
maxZoom: 10,
minZoom: 0.1,
Expand Down Expand Up @@ -2309,6 +2319,19 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
ICON_THUMBNAILS_TOGGLE,
);
});

it('should not add the find controls if find is disabled', () => {
stubs.isFindDisabled.returns(true);

docBase.bindControlListeners();

expect(docBase.controls.add).not.to.be.calledWith(
__('toggle_findbar'),
sinon.match.func,
'bp-toggle-findbar-icon',
ICON_SEARCH,
);
});
});

describe('toggleThumbnails()', () => {
Expand Down
25 changes: 25 additions & 0 deletions src/lib/viewers/doc/__tests__/DocFindBar-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -584,4 +584,29 @@ describe('lib/viewers/doc/DocFindBar', () => {
expect(stubs.add).to.be.calledWith(CLASS_HIDDEN);
});
});

describe('toggle()', () => {
beforeEach(() => {
stubs.open = sandbox.stub(docFindBar, 'open');
stubs.close = sandbox.stub(docFindBar, 'close');
});

it('should open if not currently opened', () => {
docFindBar.opened = false;

docFindBar.toggle();

expect(docFindBar.open).to.be.called;
expect(docFindBar.close).not.to.be.called;
});

it('should close if currently opened', () => {
docFindBar.opened = true;

docFindBar.toggle();

expect(docFindBar.open).not.to.be.called;
expect(docFindBar.close).to.be.called;
});
});
});