fix: honor back/next state on page drops correctly (#636)

src/container/page-list/page-list-container.tsx

@@ -15,9 +15,7 @@ import * as utils from '../../alva-util';
 @MobxReact.inject('store')
 @MobxReact.observer
 export class PageListContainer extends React.Component {
-	private dropTargetIndex: number;
-	private draggedIndex: number;
-	private draggedPage: Page;
+	private draggedPage?: Page;
 
 	private handleDragStart(e: React.DragEvent<HTMLElement>): void {
 		const { store } = this.props as { store: Store.ViewStore };
@@ -28,48 +26,60 @@ export class PageListContainer extends React.Component {
 		}
 
 		this.draggedPage = draggedPage;
-		this.draggedIndex = store.getProject().getPageIndex(draggedPage);
 		e.dataTransfer.effectAllowed = 'copy';
 	}
 
 	private handleDragLeave(e: React.DragEvent<HTMLElement>): void {
 		const { store } = this.props as { store: Store.ViewStore };
 		const validDropTarget = utils.pageFromTarget(e.target, store);
+
 		if (!validDropTarget) {
 			e.preventDefault();
 			return;
 		}
+
 		validDropTarget.setDroppableBackState(false);
 		validDropTarget.setDroppableNextState(false);
 	}
 
 	private handleDragOver(e: React.DragEvent<HTMLElement>): void {
+		e.preventDefault();
+
 		const { store } = this.props as { store: Store.ViewStore };
-		const validDropTarget = utils.pageFromTarget(e.target, store);
+		const dropTarget = utils.pageFromTarget(e.target, store);
 
-		if (!validDropTarget) {
-			e.preventDefault();
+		if (!dropTarget || !this.draggedPage) {
 			return;
 		}
-		this.dropTargetIndex = store.getProject().getPageIndex(validDropTarget);
-		if (this.draggedIndex >= this.dropTargetIndex) {
-			validDropTarget.setDroppableBackState(true);
-		} else {
-			validDropTarget.setDroppableNextState(true);
-		}
 
-		e.dataTransfer.dropEffect = 'copy';
+		if (this.draggedPage.getIndex() > dropTarget.getIndex()) {
+			dropTarget.setDroppableBackState(true);
+			e.dataTransfer.dropEffect = 'copy';
+		} else if (this.draggedPage.getIndex() < dropTarget.getIndex()) {
+			dropTarget.setDroppableNextState(true);
+			e.dataTransfer.dropEffect = 'copy';
+		}
 	}
 
 	private handleDrop(e: React.DragEvent<HTMLElement>): void {
 		const { store } = this.props as { store: Store.ViewStore };
 		const project = store.getProject();
+		const dropTarget = utils.pageFromTarget(e.target, store);
+
+		if (!dropTarget || !this.draggedPage) {
+			this.draggedPage = undefined;
+			e.preventDefault();
+			return;
+		}
+
+		if (dropTarget.getPageDropState().next) {
+			project.movePageAfter({ page: this.draggedPage, targetPage: dropTarget });
+			dropTarget.setDroppableNextState(false);
+		} else if (dropTarget.getPageDropState().back) {
+			project.movePageBefore({ page: this.draggedPage, targetPage: dropTarget });
+			dropTarget.setDroppableBackState(false);
+		}
 
-		project.getPages().forEach((page: Page) => {
-			page.setDroppableBackState(false);
-			page.setDroppableNextState(false);
-		});
-		project.reArrangePagesIndex(this.dropTargetIndex, this.draggedPage);
 		store.commit();
 	}
 

src/model/page/page.ts

@@ -41,8 +41,6 @@ export class Page {
 	@Mobx.observable private name: string = 'Page';
 	@Mobx.observable public nameState: Types.EditableTitleState = Types.EditableTitleState.Editable;
 
-	// @Mobx.observable public isdroppable: boolean = false;
-
 	/**
 	 * UI property flags to highlight the area
 	 * where the page can be dropped.
@@ -228,6 +226,10 @@ export class Page {
 		return this.id;
 	}
 
+	public getIndex(): number {
+		return this.project.getPages().indexOf(this);
+	}
+
 	public getName(options?: { unedited: boolean }): string {
 		if ((!options || !options.unedited) && this.nameState === Types.EditableTitleState.Editing) {
 			return this.editedName;
@@ -258,22 +260,14 @@ export class Page {
 		this.active = active;
 	}
 
-	/**
-	 * Sets the value of the current state for
-	 * the back page droppable area.
-	 */
 	@Mobx.action
-	public setDroppableBackState(isdroppable: boolean): void {
-		this.droppablePageIndex.back = isdroppable;
+	public setDroppableBackState(droppable: boolean): void {
+		this.droppablePageIndex.back = droppable;
 	}
 
-	/**
-	 * Sets the value of the current state for
-	 * the next page droppable area.
-	 */
 	@Mobx.action
-	public setDroppableNextState(isdroppable: boolean): void {
-		this.droppablePageIndex.next = isdroppable;
+	public setDroppableNextState(droppable: boolean): void {
+		this.droppablePageIndex.next = droppable;
 	}
 
 	@Mobx.action

src/model/project.ts

@@ -383,7 +383,7 @@ export class Project {
 			return;
 		}
 
-		const nextIndex = this.getPageIndex(page) + 1;
+		const nextIndex = page.getIndex() + 1;
 
 		if (typeof nextIndex !== 'number' || Number.isNaN(nextIndex)) {
 			return;
@@ -452,10 +452,6 @@ export class Project {
 		return this.pages.find(page => page.getId() === id);
 	}
 
-	public getPageIndex(page: Page): number {
-		return this.pages.indexOf(page);
-	}
-
 	public getPages(): Page[] {
 		return this.pages;
 	}
@@ -503,7 +499,7 @@ export class Project {
 			return;
 		}
 
-		const previousIndex = this.getPageIndex(page) - 1;
+		const previousIndex = page.getIndex() - 1;
 
 		if (typeof previousIndex !== 'number' || Number.isNaN(previousIndex)) {
 			return;
@@ -564,20 +560,28 @@ export class Project {
 
 	@Mobx.action
 	public movePageAfter(opts: { page: Page; targetPage: Page }): void {
+		this.movePage({ ...opts, offset: 1 });
+	}
+
+	@Mobx.action
+	public movePageBefore(opts: { page: Page; targetPage: Page }): void {
+		this.movePage({ ...opts, offset: 0 });
+	}
+
+	@Mobx.action
+	public movePage(opts: { page: Page; targetPage: Page; offset: number }): void {
 		const targetAvailable = this.pages.some(p => p.getId() === opts.targetPage.getId());
+		const pageAvailable = this.pages.some(p => p.getId() === opts.page.getId());
 
-		if (!targetAvailable) {
+		if (!targetAvailable || !pageAvailable) {
 			return;
 		}
 
 		const index = this.pages.findIndex(p => opts.page.getId() === p.getId());
-
-		if (index > -1) {
-			this.pages.splice(index, 1);
-		}
+		this.pageList.splice(index, 1);
 
 		const targetIndex = this.pages.findIndex(p => opts.targetPage.getId() === p.getId());
-		this.pages.splice(targetIndex + 1, 0, opts.page);
+		this.pageList.splice(targetIndex + opts.offset, 0, opts.page.getId());
 	}
 
 	@Mobx.action
@@ -675,8 +679,7 @@ export class Project {
 			return false;
 		}
 
-		const pageIndex = this.getPageIndex(page);
-		this.pageList.splice(pageIndex, 1);
+		this.pageList.splice(page.getIndex(), 1);
 
 		this.pageList.splice(position, 0, page.getId());
 		return true;

src/store/view-store.ts

@@ -695,7 +695,7 @@ export class ViewStore {
 
 	@Mobx.action
 	public removePage(page: Model.Page): void {
-		const index = this.project.getPageIndex(page);
+		const index = page.getIndex();
 
 		if (this.project.getPages().length > 1) {
 			if (index !== 0) {