-
Notifications
You must be signed in to change notification settings - Fork 22
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
Rotate panes #496
base: master
Are you sure you want to change the base?
Rotate panes #496
Changes from all commits
abcaef4
4e66f34
19f24e0
d58d97c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -363,4 +363,60 @@ export class Layout extends Cell { | |||||||||||||||||||||
}) | ||||||||||||||||||||||
return cells | ||||||||||||||||||||||
} | ||||||||||||||||||||||
changeDir(){ | ||||||||||||||||||||||
if(this.dir === "TBD") return; | ||||||||||||||||||||||
const parentDir = this.layout?.dir | ||||||||||||||||||||||
const newDir = this.dir === 'rightleft' ? 'topbottom' : 'rightleft' | ||||||||||||||||||||||
let keepXoff = false; | ||||||||||||||||||||||
let keepYoff = false; | ||||||||||||||||||||||
let cellSize; | ||||||||||||||||||||||
console.log('origdir',this.dir) | ||||||||||||||||||||||
if(this.dir === 'topbottom'){ | ||||||||||||||||||||||
keepYoff = true | ||||||||||||||||||||||
cellSize = this.sy / this.cells.length | ||||||||||||||||||||||
}else if(this.dir === 'rightleft') { | ||||||||||||||||||||||
keepXoff = true | ||||||||||||||||||||||
cellSize = this.sx / this.cells.length | ||||||||||||||||||||||
} | ||||||||||||||||||||||
this.cells.forEach((c, i) => { | ||||||||||||||||||||||
const oldXoff = c.xoff | ||||||||||||||||||||||
const oldYoff = c.yoff | ||||||||||||||||||||||
const oldSx = c.sx; | ||||||||||||||||||||||
const oldSy = c.sy; | ||||||||||||||||||||||
c.yoff = oldXoff | ||||||||||||||||||||||
c.xoff = oldYoff | ||||||||||||||||||||||
c.sx = oldSy | ||||||||||||||||||||||
c.sy = oldSx | ||||||||||||||||||||||
// console.log('c',{ | ||||||||||||||||||||||
// xoff: c.xoff, | ||||||||||||||||||||||
// yoff: c.yoff, | ||||||||||||||||||||||
// sx: c.sx, | ||||||||||||||||||||||
// sy: c.sy, | ||||||||||||||||||||||
// }) | ||||||||||||||||||||||
if(keepYoff){ | ||||||||||||||||||||||
c.yoff = this.yoff + cellSize * i | ||||||||||||||||||||||
c.xoff = this.xoff | ||||||||||||||||||||||
c.sy = cellSize | ||||||||||||||||||||||
c.sx = this.sx | ||||||||||||||||||||||
}else if(keepXoff){ | ||||||||||||||||||||||
c.yoff = this.yoff | ||||||||||||||||||||||
c.xoff = this.xoff + cellSize * i | ||||||||||||||||||||||
c.sy = this.sy | ||||||||||||||||||||||
c.sx = cellSize | ||||||||||||||||||||||
} | ||||||||||||||||||||||
// console.log('c2',{ | ||||||||||||||||||||||
// xoff: c.xoff, | ||||||||||||||||||||||
// yoff: c.yoff, | ||||||||||||||||||||||
// sx: c.sx, | ||||||||||||||||||||||
// sy: c.sy, | ||||||||||||||||||||||
// }) | ||||||||||||||||||||||
}); | ||||||||||||||||||||||
this.dir = newDir | ||||||||||||||||||||||
this.refreshDividers() | ||||||||||||||||||||||
if(this.dir === parentDir){ | ||||||||||||||||||||||
const replaceIndex = this.layout.cells.findIndex(c => c.id === this.id) | ||||||||||||||||||||||
this.layout.cells.splice(replaceIndex, 1, ...this.cells) | ||||||||||||||||||||||
this.layout.refreshDividers() | ||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+416
to
+420
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prevent potential runtime error when In the conditional block starting at line 416, accessing Apply this diff to add a null check: this.dir = newDir
this.refreshDividers()
- if(this.dir === parentDir){
+ if(this.dir === parentDir && this.layout){
const replaceIndex = this.layout.cells.findIndex(c => c.id === this.id)
this.layout.cells.splice(replaceIndex, 1, ...this.cells)
this.layout.refreshDividers()
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+366
to
+421
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Recommend adding unit tests for the new The
|
||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -563,23 +563,52 @@ export class Pane extends Cell { | |||||||||||||||||||||||||||||||||||
f = () => this.gate.newTab() | ||||||||||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||||||||||
case "r": | ||||||||||||||||||||||||||||||||||||
f = () => this.gate.reset() | ||||||||||||||||||||||||||||||||||||
if(ev.shiftKey){ | ||||||||||||||||||||||||||||||||||||
this.layout.cells.forEach(c => console.log(c)) | ||||||||||||||||||||||||||||||||||||
f = () => this.layout.changeDir() | ||||||||||||||||||||||||||||||||||||
Comment on lines
+566
to
+568
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove debugging The Apply this diff to remove the unnecessary if (ev.shiftKey) {
- this.layout.cells.forEach(c => console.log(c));
f = () => this.layout.changeDir();
} else {
f = () => this.gate.reset();
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
}else { | ||||||||||||||||||||||||||||||||||||
f = () => this.gate.reset() | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||||||||||
// this key is at terminal level | ||||||||||||||||||||||||||||||||||||
case "l": | ||||||||||||||||||||||||||||||||||||
f = () => this.t7.map.showLog() | ||||||||||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||||||||||
case "ArrowLeft": | ||||||||||||||||||||||||||||||||||||
f = () => this.w.moveFocus("left") | ||||||||||||||||||||||||||||||||||||
if(ev.shiftKey){ | ||||||||||||||||||||||||||||||||||||
const pane = this.w.getPane("left") | ||||||||||||||||||||||||||||||||||||
if(!pane) return; | ||||||||||||||||||||||||||||||||||||
this.w.swapPanes(this,pane) | ||||||||||||||||||||||||||||||||||||
}else { | ||||||||||||||||||||||||||||||||||||
f = () => this.w.moveFocus("left") | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+578
to
+584
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Provide user feedback when no pane exists to swap with on Shift+ArrowLeft When Shift+ArrowLeft is pressed and there is no pane to the left, the function returns silently. Consider notifying the user to improve the user experience. Apply this diff to inform the user when no adjacent pane is available: if (ev.shiftKey) {
const pane = this.w.getPane("left");
if (!pane) {
+ this.gate.notify("No pane to the left to swap with.");
return;
}
this.w.swapPanes(this, pane);
} else {
f = () => this.w.moveFocus("left");
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||||||||||
case "ArrowRight": | ||||||||||||||||||||||||||||||||||||
f = () => this.w.moveFocus("right") | ||||||||||||||||||||||||||||||||||||
if(ev.shiftKey){ | ||||||||||||||||||||||||||||||||||||
const pane = this.w.getPane("right") | ||||||||||||||||||||||||||||||||||||
if(!pane) return; | ||||||||||||||||||||||||||||||||||||
this.w.swapPanes(this,pane) | ||||||||||||||||||||||||||||||||||||
}else { | ||||||||||||||||||||||||||||||||||||
f = () => this.w.moveFocus("right") | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+587
to
+593
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Provide user feedback when no pane exists to swap with on Shift+ArrowRight Similar to the left direction, inform the user when there's no pane to the right to swap with. Apply this diff: if (ev.shiftKey) {
const pane = this.w.getPane("right");
if (!pane) {
+ this.gate.notify("No pane to the right to swap with.");
return;
}
this.w.swapPanes(this, pane);
} else {
f = () => this.w.moveFocus("right");
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||||||||||
case "ArrowUp": | ||||||||||||||||||||||||||||||||||||
f = () => this.w.moveFocus("up") | ||||||||||||||||||||||||||||||||||||
if(ev.shiftKey){ | ||||||||||||||||||||||||||||||||||||
const pane = this.w.getPane("up") | ||||||||||||||||||||||||||||||||||||
if(!pane) return; | ||||||||||||||||||||||||||||||||||||
this.w.swapPanes(this,pane) | ||||||||||||||||||||||||||||||||||||
}else { | ||||||||||||||||||||||||||||||||||||
f = () => this.w.moveFocus("up") | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+596
to
+602
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Provide user feedback when no pane exists to swap with on Shift+ArrowUp When swapping panes upward, notify the user if no pane is available above. Apply this diff: if (ev.shiftKey) {
const pane = this.w.getPane("up");
if (!pane) {
+ this.gate.notify("No pane above to swap with.");
return;
}
this.w.swapPanes(this, pane);
} else {
f = () => this.w.moveFocus("up");
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||||||||||
case "ArrowDown": | ||||||||||||||||||||||||||||||||||||
f = () => this.w.moveFocus("down") | ||||||||||||||||||||||||||||||||||||
if(ev.shiftKey){ | ||||||||||||||||||||||||||||||||||||
const pane = this.w.getPane("down") | ||||||||||||||||||||||||||||||||||||
if(!pane) return; | ||||||||||||||||||||||||||||||||||||
this.w.swapPanes(this,pane) | ||||||||||||||||||||||||||||||||||||
}else { | ||||||||||||||||||||||||||||||||||||
f = () => this.w.moveFocus("down") | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+605
to
+611
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Provide user feedback when no pane exists to swap with on Shift+ArrowDown When swapping panes downward, inform the user if there's no pane below. Apply this diff: if (ev.shiftKey) {
const pane = this.w.getPane("down");
if (!pane) {
+ this.gate.notify("No pane below to swap with.");
return;
}
this.w.swapPanes(this, pane);
} else {
f = () => this.w.moveFocus("down");
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||||||||||
case "p": | ||||||||||||||||||||||||||||||||||||
f = () => this.t7.dumpLog() | ||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -182,7 +182,7 @@ export class Window { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (this.rootLayout) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.rootLayout.fit() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
moveFocus(where) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
getPane(where){ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const a = this.activeP, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
b = a.t.buffer.active, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
x = a.xoff + b.cursorX * a.sx / a.t.cols, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -213,11 +213,21 @@ export class Window { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (match(c)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
nextPane = c | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return nextPane | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
moveFocus(where) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const nextPane = this.getPane(where) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (nextPane) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
nextPane.focus() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this.gate.sendState() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
switchPane(where){ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.log(this.activeP) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+227
to
+229
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implement functionality for The Consider implementing the pane-switching logic or, if this is a placeholder, add a TODO comment to indicate future work: switchPane(where){
// TODO: Implement pane-switching logic based on the 'where' parameter
console.log(this.activeP);
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @daonb, happy to help! Here's a possible implementation for the switchPane(where: string) {
const paneToSwap = this.getPane(where);
if (paneToSwap) {
this.swapPanes(this.activeP, paneToSwap);
this.gate.sendState();
} else {
console.log(`No pane found in the direction: ${where}`);
}
} 😊🐰 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
µµ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
updateDivideButtons() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const bV = document.getElementById("divide-v") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const bH = document.getElementById("divide-h") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -265,5 +275,33 @@ export class Window { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
newLayout.refreshDividers() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return newLayout | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
swapPanes(pane1: Pane, pane2: Pane){ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const pane1Layout = pane1.layout | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const pane2Layout = pane2.layout | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const pane1Index = pane1Layout.cells.indexOf(pane1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const pane2Index = pane2Layout.cells.indexOf(pane2); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane1Layout.cells[pane1Index] = pane2; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane2Layout.cells[pane2Index] = pane1; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane1.layout = pane2Layout; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane2.layout = pane1Layout; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const pane1Xoff = pane1.xoff | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const pane1Yoff = pane1.yoff | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const pane1Sx = pane1.sx | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const pane1Sy = pane1.sy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane1.xoff = pane2.xoff | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane1.yoff = pane2.yoff | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane1.sx = pane2.sx | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane1.sy = pane2.sy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane2.xoff = pane1Xoff | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane2.yoff = pane1Yoff | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane2.sx = pane1Sx | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane2.sy = pane1Sy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+288
to
+302
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Refactor property swapping to reduce code duplication The swapping of Consider simplifying the code using array destructuring: // Swap position and size properties
- const pane1Xoff = pane1.xoff
- const pane1Yoff = pane1.yoff
- const pane1Sx = pane1.sx
- const pane1Sy = pane1.sy
-
- pane1.xoff = pane2.xoff
- pane1.yoff = pane2.yoff
- pane1.sx = pane2.sx
- pane1.sy = pane2.sy
-
- pane2.xoff = pane1Xoff
- pane2.yoff = pane1Yoff
- pane2.sx = pane1Sx
- pane2.sy = pane1Sy
+ [pane1.xoff, pane2.xoff] = [pane2.xoff, pane1.xoff];
+ [pane1.yoff, pane2.yoff] = [pane2.yoff, pane1.yoff];
+ [pane1.sx, pane2.sx] = [pane2.sx, pane1.sx];
+ [pane1.sy, pane2.sy] = [pane2.sy, pane1.sy]; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane1Layout.refreshDividers(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pane2Layout.refreshDividers(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+278
to
+305
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle swapping panes within the same layout to prevent errors The Add a condition to handle cases where panes share the same layout: swapPanes(pane1: Pane, pane2: Pane){
const pane1Layout = pane1.layout
const pane2Layout = pane2.layout
const pane1Index = pane1Layout.cells.indexOf(pane1);
const pane2Index = pane2Layout.cells.indexOf(pane2);
+ if (pane1Layout === pane2Layout) {
+ // Swap panes within the same layout
+ [pane1Layout.cells[pane1Index], pane1Layout.cells[pane2Index]] = [pane2, pane1];
+ } else {
pane1Layout.cells[pane1Index] = pane2;
pane2Layout.cells[pane2Index] = pane1;
+ }
pane1.layout = pane2Layout;
pane2.layout = pane1Layout;
// Rest of the code remains the same
} 📝 Committable suggestion
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we assume both panes are of the same layout. It does make sense to check it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @daonb, understood. Noting that the code assumes both panes are within the same layout makes sense. Adding the check will help ensure proper handling. (^_^) ✏️ Learnings added
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// used to recieve the touch event from the gate | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle case where
findIndex
returns -1 to avoid unexpected behaviorAt lines 417-418,
findIndex
may return-1
if the current layout'sid
is not found inthis.layout.cells
. Using-1
as the start index insplice
could remove the last element of the array unintentionally. Ensure thatreplaceIndex
is valid before proceeding with the splice operation.Apply this diff to check the validity of
replaceIndex
:📝 Committable suggestion