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

Switch to new page number display format #1366

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
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
64 changes: 53 additions & 11 deletions src/BookReader/Navbar/Navbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,43 @@
* Switch navbar controls on mobile and desktop
*/
switchNavbarControls() {
// we don't want navbar controls switching with liner-notes
if (this.br.options.bookType !== 'linerNotes') {
if (this.br.refs.$brContainer.prop('clientWidth') < 640) {
if (this.br.refs.$brContainer.prop('clientWidth') < 640) {
this.showMinimumNavPageNum();
// we don't want navbar controls switching with liner-notes
if (this.br.options.bookType !== 'linerNotes') {
this.showMinimumNavbarControls();
} else {
}
} else {
this.showMaximumNavPageNum();

Check warning on line 148 in src/BookReader/Navbar/Navbar.js

View check run for this annotation

Codecov / codecov/patch

src/BookReader/Navbar/Navbar.js#L148

Added line #L148 was not covered by tests
// we don't want navbar controls switching with liner-notes
if (this.br.options.bookType !== 'linerNotes') {
this.showMaximumNavbarControls();
}
}
}

/**
* Switch Book Nav page number display to minimum/mobile
*/
showMinimumNavPageNum() {
const minElement = document.querySelector('.BRcurrentpage.BRmin');
const maxElement = document.querySelector('.BRcurrentpage.BRmax');

if (minElement) minElement.classList.remove('hide');
if (maxElement) maxElement.classList.add('hide');
}

/**
* Switch Book Nav page number display to maximum/desktop
*/
showMaximumNavPageNum() {
const minElement = document.querySelector('.BRcurrentpage.BRmin');
const maxElement = document.querySelector('.BRcurrentpage.BRmax');

Check warning on line 172 in src/BookReader/Navbar/Navbar.js

View check run for this annotation

Codecov / codecov/patch

src/BookReader/Navbar/Navbar.js#L170-L172

Added lines #L170 - L172 were not covered by tests

if (minElement) minElement.classList.add('hide');
if (maxElement) maxElement.classList.remove('hide');
}

/**
* Switch Book Navbar controls to minimised
* NOTE: only `this.minimumControls` and `this.maximumControls` switch on resize
Expand Down Expand Up @@ -203,7 +230,10 @@
<div class="BRpager"></div>
<div class="BRnavline"></div>
</div>
<p><span class='BRcurrentpage'></span></p>
<p>
<span class="BRcurrentpage BRmax"></span>
<span class="BRcurrentpage BRmin"></span>
</p>
</li>
${this._renderControls()}
</ul>
Expand Down Expand Up @@ -246,9 +276,10 @@
/**
* Returns the textual representation of the current page for the navbar
* @param {number} index
* @param {boolean} [useMaxFormat = false]
* @return {string}
*/
getNavPageNumString(index) {
getNavPageNumString(index, useMaxFormat = false) {
const { br } = this;
// Accessible index starts at 0 (alas) so we add 1 to make human
const pageNum = br.book.getPageNum(index);
Expand All @@ -268,15 +299,17 @@
this.maxPageNum = maxPageNum;
}

return getNavPageNumHtml(index, numLeafs, pageNum, pageType, this.maxPageNum);
return getNavPageNumHtml(index, numLeafs, pageNum, pageType, this.maxPageNum, useMaxFormat);

}

/**
* Renders the navbar string to the DOM
* @param {number} index
*/
updateNavPageNum(index) {
this.$root.find('.BRcurrentpage').html(this.getNavPageNumString(index));
this.$root.find('.BRcurrentpage.BRmax').html(this.getNavPageNumString(index, true));
this.$root.find('.BRcurrentpage.BRmin').html(this.getNavPageNumString(index));
}

/**
Expand All @@ -298,14 +331,23 @@
* @param {number|string} pageNum
* @param {*} pageType - Deprecated
* @param {number} maxPageNum
* @param {boolean} [useMaxFormat = false]
* @return {string}
*/
export function getNavPageNumHtml(index, numLeafs, pageNum, pageType, maxPageNum) {
export function getNavPageNumHtml(index, numLeafs, pageNum, pageType, maxPageNum, useMaxFormat = false) {
const pageIsAsserted = pageNum[0] != 'n';
const pageIndex = index + 1;

if (!pageIsAsserted) {
pageNum = '—';
}

if (useMaxFormat === true) {
pezvi marked this conversation as resolved.
Show resolved Hide resolved
return `Page ${pageNum} (${pageIndex}/${numLeafs})`;
}

if (!pageIsAsserted) {
const pageIndex = index + 1;
return `(${pageIndex} of ${numLeafs})`; // Page (8 of 10)
return `(${pageIndex} of ${numLeafs})`;
}

const bookLengthLabel = (maxPageNum && parseFloat(pageNum)) ? ` of ${maxPageNum}` : '';
Expand Down
3 changes: 0 additions & 3 deletions src/css/_BRnav.scss
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,6 @@
display: block;
}
}
&.hide {
display: none;
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/css/_controls.scss
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@
height: 30px;
}

.controls .hide {
display: none;
}

@keyframes slideUp {
from {
opacity: 0;
Expand Down
19 changes: 16 additions & 3 deletions tests/jest/BookReader/Navbar/Navbar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,30 @@ import BookReader from '@/src/BookReader.js';

describe('getNavPageNumHtml', () => {
const f = getNavPageNumHtml;
test('handle n-prefixed page numbers', () => {

test('handle n-prefixed page numbers-min format', () => {
expect(f(3, 40, 'n3', '', 40)).toBe('(4 of 40)');
});

test('handle regular page numbers', () => {
test('handle regular page numbers-min format', () => {
expect(f(3, 40, '14', '', 40)).toBe('14 of 40');
});

test('handle no max page', () => {
test('handle no max page-min format', () => {
expect(f(3, 40, '14', '', null)).toBe('14');
});

test('handle n-prefixed page numbers-max format', () => {
pezvi marked this conversation as resolved.
Show resolved Hide resolved
expect(f(3, 40, 'n3', '', 40, true)).toBe('Page — (4/40)');
});

test('handle regular page numbers-max format', () => {
expect(f(3, 40, '14', '', 40, true)).toBe('Page 14 (4/40)');
});

test('handle no max page-max format', () => {
expect(f(3, 40, '14', '', null, true)).toBe('Page 14 (4/40)');
});
});

/** @type {BookReader} */
Expand Down
Loading