DRY up Mode2Up.flipLeftToRight and Mode2Up.flipRightToLeft
These functions have very similar logic, and are largely duplicates of each other. DRY them up.
Summary of Requirements
- [ ] Create pre-refactor unit tests for
Mode2Up.flipLeftToRightandMode2Up.flipRightToLeft - [ ]
Mode2Up.flipLeftToRightandMode2Up.flipRightToLeftshould still exist, but call a helper function which takes more parameters and reduces code duplication - [ ] Update tests and ensure they still pass
I can do this @cdrini I need some more details on it like how exactly you want to refactor?
@cdrini is this still open? I'd like to work on it.
@cdrini If the issue is still not resolved yet would like to take it up from here on.
@shubham00jain @diwanshu8850 are you working on this
@cdrini @iisa can you please check I have refactored the code If you gave it a go I will send a pull request and start with the writing tests for this helper function
helperflipper(newIndexL, newIndexR, LeftToRight){
this.br.refs.$brContainer.addClass("BRpageFlipping");
const leftLeaf = this.br.twoPage.currentIndexL;
const rightLeaf = this.br.twoPage.currentIndexR;
const oldLeafEdgeWidthL = this.br.leafEdgeWidth(leftLeaf);
const newLeafEdgeWidthL = this.br.leafEdgeWidth(newIndexL);
const oldLeafEdgeWidthR = this.br.twoPage.edgeWidth - oldLeafEdgeWidthL;
const newLeafEdgeWidthR = this.br.twoPage.edgeWidth - newLeafEdgeWidthL;
const currWidthL = this.getPageWidth(leftLeaf);
const currWidthR = this.getPageWidth(rightLeaf);
const newWidthL = this.getPageWidth(newIndexL);
const newWidthR = this.getPageWidth(newIndexR);
const top = this.top();
const middle = this.br.twoPage.middle;
let leafEdgeTmpWL;
let leafEdgeTmpL;
const speed = this.br.flipSpeed;
const gutter = this.br.twoPage.middle + this.gutterOffsetForIndex(newIndexL);
if (LeftToRight) {
leafEdgeTmpWL = gutter - currWidthL - oldLeafEdgeWidthL - newLeafEdgeWidthL;
leafEdgeTmpL = gutter + newWidthR;
} else {
leafEdgeTmpWL = gutter + newWidthR;
leafEdgeTmpL = gutter - newWidthL - leafEdgeTmpW;
}
const $twoPageViewEl = this.br.refs.$brTwoPageView;
this.br.leafEdgeTmp = document.createElement('div');
this.br.leafEdgeTmp.className = 'BRleafEdgeTmp';
$(this.br.leafEdgeTmp).css({
width: `${leafEdgeTmpW}px`,
height: `${this.br.twoPage.height}px`,
left: `${leafEdgeTmpWL}px`,
top: `${top}px`,
zIndex: 1000,
}).appendTo($twoPageViewEl);
let oldFlipFromLeafIndex;
let oldFlipToLeafIndex;
let newFlipFromLeafIndex;
let newFlipToLeafIndex;
if (LeftToRight) {
$(this.leafEdgeL).css({
width: `${newLeafEdgeWidthL}px`,
left: `${leafEdgeTmpWL}px`
});
const left = $(this.br.prefetchedImgs[leftLeaf]).offset().left;
const right = `${$twoPageViewEl.prop('clientWidth') - left - $(this.br.prefetchedImgs[leftLeaf]).width() + $twoPageViewEl.offset().left - 2}px`;
$(this.br.prefetchedImgs[leftLeaf]).css({
right: right,
left: ''
});
oldFlipFromLeafIndex=leftLeaf;
oldFlipToLeafIndex=rightLeaf;
newFlipFromLeafIndex=newIndexL;
newFlipToLeafIndex=newIndexR;
} else {
$(this.leafEdgeR).css({
width: `${newLeafEdgeWidthR}px`,
left: `${leafEdgeTmpWL}px`
});
oldFlipFromLeafIndex=rightLeaf;
oldFlipToLeafIndex=leftLeaf;
newFlipFromLeafIndex=newIndexR;
newFlipToLeafIndex=newIndexL;
}
$(this.br.leafEdgeTmp).animate({ left: gutter }, speed, 'easeInSine');
if (this.br.enableSearch) this.br.removeSearchHilites();
$(this.br.leafEdgeTmp).animate({ left: `${leafEdgeTmpL}px` }, speed, 'easeOutSine');
$(this.br.prefetchedImgs[oldFlipFromLeafIndex]).animate({ width: '0px' }, speed, 'easeInSine', () => {
this.br.$('.BRgutter').css({ left: `${gutter - this.br.twoPage.bookSpineDivWidth * 0.5}px` });
$(this.br.prefetchedImgs[newFlipToLeafIndex]).animate({ width: `${newWidthR}px` }, speed, 'easeOutSine', () => {
$(this.br.prefetchedImgs[newFlipFromLeafIndex]).css('zIndex', 2);
//jquery adds display:block to the element style, which interferes with our print css
$(this.br.prefetchedImgs[newFlipFromLeafIndex]).css('display', '');
$(this.br.prefetchedImgs[newFlipToLeafIndex]).css('display', '');
$(this.leafEdgeR).css({
// Moves the right leaf edge
width: `${this.br.twoPage.edgeWidth - newLeafEdgeWidthL}px`,
left: `${gutter + newWidthR}px`
});
$(this.leafEdgeL).css({
// Moves and resizes the left leaf edge
width: `${newLeafEdgeWidthL}px`,
left: `${gutter - newWidthL - newLeafEdgeWidthL}px`
});
// Resizes the brown border div
$(this.br.twoPage.coverDiv).css({
width: `${this.coverWidth(newWidthL + newWidthR)}px`,
left: `${gutter - newWidthL - newLeafEdgeWidthL - this.br.twoPage.coverInternalPadding}px`
});
$(this.br.leafEdgeTmp).remove();
this.br.leafEdgeTmp = null;
// $$$ TODO refactor with opposite direction flip
this.br.twoPage.currentIndexL = newFlipFromLeafIndex;
this.br.twoPage.currentIndexR = newFlipToLeafIndex;
this.br.twoPage.scaledWL = newWidthL;
this.br.twoPage.scaledWR = newWidthR;
this.br.twoPage.gutter = gutter;
this.br.updateFirstIndex(this.br.twoPage.currentIndexL);
this.br.displayedIndices = [newFlipFromLeafIndex, newFlipToLeafIndex];
this.br.pruneUnusedImgs();
this.br.prefetch();
this.br.animating = false;
if (this.br.enableSearch) this.br.updateSearchHilites();
this.setMouseHandlers();
if (this.br.animationFinishedCallback) {
this.br.animationFinishedCallback();
this.br.animationFinishedCallback = null;
}
this.br.refs.$brContainer.removeClass("BRpageFlipping");
this.br.textSelectionPlugin?.stopPageFlip(this.br.refs.$brContainer);
this.br.trigger('pageChanged');
});
});
}
Hey, I would like to work on this issue. Please assign me to it.