From 2179ce6fff78d214344b703c16be625dbf314b81 Mon Sep 17 00:00:00 2001 From: Nivedin <53208152+nivedin@users.noreply.github.com> Date: Tue, 14 Mar 2023 14:01:47 +0530 Subject: [PATCH] fix: reordering bugs and UX fixes (#2948) --- .../hoppscotch-common/src/components.d.ts | 1 - .../src/components/collections/Collection.vue | 118 ++++++--- .../components/collections/MyCollections.vue | 4 + .../src/components/collections/Request.vue | 127 +++++++--- .../collections/TeamCollections.vue | 3 + .../src/components/collections/index.vue | 229 ++++++++++-------- .../helpers/teams/TeamCollectionAdapter.ts | 24 +- .../src/newstore/collections.ts | 21 +- .../src/newstore/reordering.ts | 46 ++++ 9 files changed, 390 insertions(+), 183 deletions(-) create mode 100644 packages/hoppscotch-common/src/newstore/reordering.ts diff --git a/packages/hoppscotch-common/src/components.d.ts b/packages/hoppscotch-common/src/components.d.ts index 970a9ec0b..40471f3c5 100644 --- a/packages/hoppscotch-common/src/components.d.ts +++ b/packages/hoppscotch-common/src/components.d.ts @@ -89,7 +89,6 @@ declare module '@vue/runtime-core' { HoppSmartSpinner: typeof import('@hoppscotch/ui')['HoppSmartSpinner'] HoppSmartTab: typeof import('@hoppscotch/ui')['HoppSmartTab'] HoppSmartTabs: typeof import('@hoppscotch/ui')['HoppSmartTabs'] - HoppSmartToggle: typeof import('@hoppscotch/ui')['HoppSmartToggle'] HttpAuthorization: typeof import('./components/http/Authorization.vue')['default'] HttpBody: typeof import('./components/http/Body.vue')['default'] HttpBodyParameters: typeof import('./components/http/BodyParameters.vue')['default'] diff --git a/packages/hoppscotch-common/src/components/collections/Collection.vue b/packages/hoppscotch-common/src/components/collections/Collection.vue index 83f87c90c..f368409fd 100644 --- a/packages/hoppscotch-common/src/components/collections/Collection.vue +++ b/packages/hoppscotch-common/src/components/collections/Collection.vue @@ -4,7 +4,7 @@ class="h-1 w-full transition" :class="[ { - 'bg-accentDark': ordering && notSameDestination, + 'bg-accentDark': isReorderable, }, ]" @drop="orderUpdateCollectionEvent" @@ -20,35 +20,43 @@ }" >
- - - - - - - {{ collectionName }} + + + - + + + {{ collectionName }} + + +
, + default: null, + required: true, + }, data: { type: Object as PropType | TeamCollection>, default: () => ({}), @@ -258,6 +276,12 @@ const dragging = ref(false) const ordering = ref(false) const dropItemID = ref("") +const currentReorderingStatus = useReadonlyStream(currentReorderingStatus$, { + type: "collection", + id: "", + parentID: "", +}) + // Used to determine if the collection is being dragged to a different destination // This is used to make the highlight effect work watch( @@ -293,11 +317,52 @@ watch( } ) +const isRequestDragging = computed(() => { + return currentReorderingStatus.value.type === "request" +}) + +const isSameParent = computed(() => { + return currentReorderingStatus.value.parentID === props.parentID +}) + +const isReorderable = computed(() => { + return ( + ordering.value && + notSameDestination.value && + !isRequestDragging.value && + isSameParent.value + ) +}) + const dragStart = ({ dataTransfer }: DragEvent) => { if (dataTransfer) { emit("drag-event", dataTransfer) dropItemID.value = dataTransfer.getData("collectionIndex") dragging.value = !dragging.value + changeCurrentReorderStatus({ + type: "collection", + id: props.id, + parentID: props.parentID, + }) + } +} + +// Trigger the re-ordering event when a collection is dragged over another collection's top section +const handleDragOver = (e: DragEvent) => { + dragging.value = true + if (e.offsetY < 10 && notSameDestination.value) { + ordering.value = true + dragging.value = false + } else { + ordering.value = false + } +} + +const handelDrop = (e: DragEvent) => { + if (ordering.value) { + orderUpdateCollectionEvent(e) + } else { + dropEvent(e) } } @@ -305,8 +370,7 @@ const dropEvent = (e: DragEvent) => { if (e.dataTransfer) { e.stopPropagation() emit("drop-event", e.dataTransfer) - dragging.value = !dragging.value - dropItemID.value = "" + resetDragState() } } @@ -314,8 +378,7 @@ const orderUpdateCollectionEvent = (e: DragEvent) => { if (e.dataTransfer) { e.stopPropagation() emit("update-collection-order", e.dataTransfer) - ordering.value = !ordering.value - dropItemID.value = "" + resetDragState() } } @@ -334,6 +397,5 @@ const isCollLoading = computed(() => { const resetDragState = () => { dragging.value = false ordering.value = false - dropItemID.value = "" } diff --git a/packages/hoppscotch-common/src/components/collections/MyCollections.vue b/packages/hoppscotch-common/src/components/collections/MyCollections.vue index 7c6fdcbce..759b7ad69 100644 --- a/packages/hoppscotch-common/src/components/collections/MyCollections.vue +++ b/packages/hoppscotch-common/src/components/collections/MyCollections.vue @@ -39,6 +39,7 @@
- - - - - {{ request.method }} - - - - - {{ request.name }} + + + + + {{ request.method }} + - + + {{ request.name }} + v-if="isActive" + v-tippy="{ theme: 'tooltip' }" + class="relative h-1.5 w-1.5 flex flex-shrink-0 mx-3" + :title="`${t('collection.request_in_use')}`" + > + + + + - +
, + default: null, + required: true, + }, collectionsType: { type: String as PropType, default: "my-collections", @@ -222,6 +236,12 @@ const duplicate = ref(null) const dragging = ref(false) const ordering = ref(false) +const currentReorderingStatus = useReadonlyStream(currentReorderingStatus$, { + type: "collection", + id: "", + parentID: "", +}) + const requestMethodLabels = { get: "text-green-500", post: "text-yellow-500", @@ -255,13 +275,41 @@ const dragStart = ({ dataTransfer }: DragEvent) => { if (dataTransfer) { emit("drag-request", dataTransfer) dragging.value = !dragging.value + changeCurrentReorderStatus({ + type: "request", + id: props.requestID, + parentID: props.parentID, + }) + } +} + +const isCollectionDragging = computed(() => { + return currentReorderingStatus.value.type === "collection" +}) + +const isSameParent = computed(() => { + return currentReorderingStatus.value.parentID === props.parentID +}) + +const isReorderable = computed(() => { + return ordering.value && !isCollectionDragging.value && isSameParent.value +}) + +// Trigger the re-ordering event when a request is dragged over another request's top section +const handleDragOver = (e: DragEvent) => { + dragging.value = true + if (e.offsetY < 10) { + ordering.value = true + dragging.value = false + } else { + ordering.value = false } } const dropEvent = (e: DragEvent) => { if (e.dataTransfer) { e.stopPropagation() - ordering.value = !ordering.value + resetDragState() emit("update-request-order", e.dataTransfer) } } @@ -273,4 +321,9 @@ const isRequestLoading = computed(() => { return false } }) + +const resetDragState = () => { + dragging.value = false + ordering.value = false +} diff --git a/packages/hoppscotch-common/src/components/collections/TeamCollections.vue b/packages/hoppscotch-common/src/components/collections/TeamCollections.vue index 2b694d7fe..c26e29607 100644 --- a/packages/hoppscotch-common/src/components/collections/TeamCollections.vue +++ b/packages/hoppscotch-common/src/components/collections/TeamCollections.vue @@ -53,6 +53,7 @@ { return filteredCollections }) -const isSelected = computed(() => { - return ({ - collectionIndex, - folderPath, - requestIndex, - collectionID, - folderID, - requestID, - }: { - collectionIndex?: number | undefined - folderPath?: string | undefined - requestIndex?: number | undefined - collectionID?: string | undefined - folderID?: string | undefined - requestID?: string | undefined - }) => { - if (collectionIndex !== undefined) { - return ( - props.picked && - props.picked.pickedType === "my-collection" && - props.picked.collectionIndex === collectionIndex - ) - } else if (requestIndex !== undefined && folderPath !== undefined) { - return ( - props.picked && - props.picked.pickedType === "my-request" && - props.picked.folderPath === folderPath && - props.picked.requestIndex === requestIndex - ) - } else if (folderPath !== undefined) { - return ( - props.picked && - props.picked.pickedType === "my-folder" && - props.picked.folderPath === folderPath - ) - } else if (collectionID !== undefined) { - return ( - props.picked && - props.picked.pickedType === "teams-collection" && - props.picked.collectionID === collectionID - ) - } else if (requestID !== undefined) { - return ( - props.picked && - props.picked.pickedType === "teams-request" && - props.picked.requestID === requestID - ) - } else if (folderID !== undefined) { - return ( - props.picked && - props.picked.pickedType === "teams-folder" && - props.picked.folderID === folderID - ) - } +const isSelected = ({ + collectionIndex, + folderPath, + requestIndex, + collectionID, + folderID, + requestID, +}: { + collectionIndex?: number | undefined + folderPath?: string | undefined + requestIndex?: number | undefined + collectionID?: string | undefined + folderID?: string | undefined + requestID?: string | undefined +}) => { + if (collectionIndex !== undefined) { + return ( + props.picked && + props.picked.pickedType === "my-collection" && + props.picked.collectionIndex === collectionIndex + ) + } else if (requestIndex !== undefined && folderPath !== undefined) { + return ( + props.picked && + props.picked.pickedType === "my-request" && + props.picked.folderPath === folderPath && + props.picked.requestIndex === requestIndex + ) + } else if (folderPath !== undefined) { + return ( + props.picked && + props.picked.pickedType === "my-folder" && + props.picked.folderPath === folderPath + ) + } else if (collectionID !== undefined) { + return ( + props.picked && + props.picked.pickedType === "teams-collection" && + props.picked.collectionID === collectionID + ) + } else if (requestID !== undefined) { + return ( + props.picked && + props.picked.pickedType === "teams-request" && + props.picked.requestID === requestID + ) + } else if (folderID !== undefined) { + return ( + props.picked && + props.picked.pickedType === "teams-folder" && + props.picked.folderID === folderID + ) } -}) +} const modalLoadingState = ref(false) const exportLoading = ref(false) @@ -1023,7 +1021,7 @@ const onRemoveCollection = () => { if (collectionIndex === null) return if ( - isSelected.value({ + isSelected({ collectionIndex, }) ) { @@ -1040,7 +1038,7 @@ const onRemoveCollection = () => { if (!collectionID) return if ( - isSelected.value({ + isSelected({ collectionID, }) ) { @@ -1067,7 +1065,7 @@ const onRemoveFolder = () => { if (!folderPath) return if ( - isSelected.value({ + isSelected({ folderPath, }) ) { @@ -1084,7 +1082,7 @@ const onRemoveFolder = () => { if (!collectionID) return if ( - isSelected.value({ + isSelected({ collectionID, }) ) { @@ -1118,7 +1116,7 @@ const onRemoveRequest = () => { if (folderPath === null || requestIndex === null) return if ( - isSelected.value({ + isSelected({ folderPath, requestIndex, }) @@ -1136,7 +1134,7 @@ const onRemoveRequest = () => { if (!requestID) return if ( - isSelected.value({ + isSelected({ requestID, }) ) { @@ -1342,12 +1340,10 @@ const discardRequestChange = () => { * @param path The path of the request * @returns The index of the request */ -const pathToIndex = computed(() => { - return (path: string) => { - const pathArr = path.split("/") - return parseInt(pathArr[pathArr.length - 1]) - } -}) +const pathToLastIndex = (path: string) => { + const pathArr = path.split("/") + return parseInt(pathArr[pathArr.length - 1]) +} /** * This function is called when the user drops the request inside a collection @@ -1363,7 +1359,7 @@ const dropRequest = (payload: { if (collectionsType.value.type === "my-collections" && folderPath) { moveRESTRequest( folderPath, - pathToIndex.value(requestIndex), + pathToLastIndex(requestIndex), destinationCollectionIndex ) toast.success(`${t("request.moved")}`) @@ -1395,6 +1391,43 @@ const dropRequest = (payload: { } } +/** + * @param path The path of the collection or request + * @returns The index of the collection or request + */ +const pathToIndex = (path: string) => { + const pathArr = path.split("/") + return pathArr +} + +/** + * Used to check if the collection exist as the parent of the childrens + * @param collectionIndexDragged The index of the collection dragged + * @param destinationCollectionIndex The index of the destination collection + * @returns True if the collection exist as the parent of the childrens + */ +const checkIfCollectionIsAParentOfTheChildren = ( + collectionIndexDragged: string, + destinationCollectionIndex: string +) => { + const collectionDraggedPath = pathToIndex(collectionIndexDragged) + const destinationCollectionPath = pathToIndex(destinationCollectionIndex) + + if (collectionDraggedPath.length < destinationCollectionPath.length) { + const slicedDestinationCollectionPath = destinationCollectionPath.slice( + 0, + collectionDraggedPath.length + ) + if (isEqual(slicedDestinationCollectionPath, collectionDraggedPath)) { + return true + } else { + return false + } + } + + return false +} + /** * This function is called when the user moves the collection * to a different collection or folder @@ -1408,6 +1441,15 @@ const dropCollection = (payload: { if (!collectionIndexDragged || !destinationCollectionIndex) return if (collectionIndexDragged === destinationCollectionIndex) return if (collectionsType.value.type === "my-collections") { + if ( + checkIfCollectionIsAParentOfTheChildren( + collectionIndexDragged, + destinationCollectionIndex + ) + ) { + toast.error(`${t("team.parent_coll_move")}`) + return + } moveRESTFolder(collectionIndexDragged, destinationCollectionIndex) draggingToRoot.value = false toast.success(`${t("collection.moved")}`) @@ -1445,12 +1487,10 @@ const dropCollection = (payload: { * @param id - path of the collection * @returns boolean - true if the collection is already in the root */ -const isAlreadyInRoot = computed(() => { - return (id: string) => { - const indexPath = id.split("/").map((i) => parseInt(i)) - return indexPath.length === 1 - } -}) +const isAlreadyInRoot = (id: string) => { + const indexPath = pathToIndex(id) + return indexPath.length === 1 +} /** * This function is called when the user drops the collection @@ -1463,7 +1503,7 @@ const dropToRoot = ({ dataTransfer }: DragEvent) => { if (!collectionIndexDragged) return if (collectionsType.value.type === "my-collections") { // check if the collection is already in the root - if (isAlreadyInRoot.value(collectionIndexDragged)) { + if (isAlreadyInRoot(collectionIndexDragged)) { toast.error(`${t("collection.invalid_root_move")}`) } else { moveRESTFolder(collectionIndexDragged, null) @@ -1506,26 +1546,25 @@ const dropToRoot = ({ dataTransfer }: DragEvent) => { * @param destinationReq - path index of the destination request * @returns boolean - true if the request is being moved to the same parent */ -const isSameSameParent = computed( - () => (draggedReq: string, destinationReq: string) => { - const draggedReqIndex = draggedReq.split("/").map((i) => parseInt(i)) - const destinationReqIndex = destinationReq - .split("/") - .map((i) => parseInt(i)) +const isSameSameParent = (draggedItem: string, destinationItem: string) => { + const draggedItemIndex = pathToIndex(draggedItem) + const destinationItemIndex = pathToIndex(destinationItem) - // length of 1 means the request is in the root - if (draggedReqIndex.length === 1 && destinationReqIndex.length === 1) { - return true - } else if ( - draggedReqIndex[draggedReqIndex.length - 2] === - destinationReqIndex[destinationReqIndex.length - 2] - ) { + // length of 1 means the request is in the root + if (draggedItemIndex.length === 1 && destinationItemIndex.length === 1) { + return true + } else if (draggedItemIndex.length === destinationItemIndex.length) { + const dragedItemParent = draggedItemIndex.slice(0, -1) + const destinationItemParent = destinationItemIndex.slice(0, -1) + if (isEqual(dragedItemParent, destinationItemParent)) { return true } else { return false } + } else { + return false } -) +} /** * This function is called when the user updates the request order in a collection @@ -1553,12 +1592,12 @@ const updateRequestOrder = (payload: { if (dragedRequestIndex === destinationRequestIndex) return if (collectionsType.value.type === "my-collections") { - if (!isSameSameParent.value(dragedRequestIndex, destinationRequestIndex)) { + if (!isSameSameParent(dragedRequestIndex, destinationRequestIndex)) { toast.error(`${t("collection.different_parent")}`) } else { updateRESTRequestOrder( - pathToIndex.value(dragedRequestIndex), - pathToIndex.value(destinationRequestIndex), + pathToLastIndex(dragedRequestIndex), + pathToLastIndex(destinationRequestIndex), destinationCollectionIndex ) toast.success(`${t("request.order_changed")}`) @@ -1608,9 +1647,7 @@ const updateCollectionOrder = (payload: { if (dragedCollectionIndex === destinationCollectionIndex) return if (collectionsType.value.type === "my-collections") { - if ( - !isSameSameParent.value(dragedCollectionIndex, destinationCollectionIndex) - ) { + if (!isSameSameParent(dragedCollectionIndex, destinationCollectionIndex)) { toast.error(`${t("collection.different_parent")}`) } else { updateRESTCollectionOrder( diff --git a/packages/hoppscotch-common/src/helpers/teams/TeamCollectionAdapter.ts b/packages/hoppscotch-common/src/helpers/teams/TeamCollectionAdapter.ts index ee9617d31..aee9af86f 100644 --- a/packages/hoppscotch-common/src/helpers/teams/TeamCollectionAdapter.ts +++ b/packages/hoppscotch-common/src/helpers/teams/TeamCollectionAdapter.ts @@ -547,6 +547,15 @@ export default class NewTeamCollectionAdapter { ) } + private reorderItems = (array: unknown[], from: number, to: number) => { + const item = array.splice(from, 1)[0] + if (from < to) { + array.splice(to - 1, 0, item) + } else { + array.splice(to, 0, item) + } + } + public updateRequestOrder( dragedRequestID: string, destinationRequestID: string, @@ -570,10 +579,7 @@ export default class NewTeamCollectionAdapter { if (requestIndex === -1) return - const request = collection.requests[requestIndex] - - collection.requests.splice(requestIndex, 1) - collection.requests.splice(destinationIndex, 0, request) + this.reorderItems(collection.requests, requestIndex, destinationIndex) this.collections$.next(tree) } @@ -600,10 +606,7 @@ export default class NewTeamCollectionAdapter { // If the collection index is not found, don't update if (collectionIndex === -1) return - const collection = coll.children[collectionIndex] - - coll.children.splice(collectionIndex, 1) - coll.children.splice(destinationIndex, 0, collection) + this.reorderItems(coll.children, collectionIndex, destinationIndex) } else { // If the collection has no parent collection, it is a root collection const collectionIndex = tree.findIndex((coll) => coll.id === collectionID) @@ -615,10 +618,7 @@ export default class NewTeamCollectionAdapter { // If the collection index is not found, don't update if (collectionIndex === -1) return - const collection = tree[collectionIndex] - - tree.splice(collectionIndex, 1) - tree.splice(destinationIndex, 0, collection) + this.reorderItems(tree, collectionIndex, destinationIndex) } this.collections$.next(tree) diff --git a/packages/hoppscotch-common/src/newstore/collections.ts b/packages/hoppscotch-common/src/newstore/collections.ts index bcfe05571..0068e24db 100644 --- a/packages/hoppscotch-common/src/newstore/collections.ts +++ b/packages/hoppscotch-common/src/newstore/collections.ts @@ -45,6 +45,15 @@ function navigateToFolderWithIndexPath( return target !== undefined ? target : null } +function reorderItems(array: unknown[], from: number, to: number) { + const item = array.splice(from, 1)[0] + if (from < to) { + array.splice(to - 1, 0, item) + } else { + array.splice(to, 0, item) + } +} + const restCollectionDispatchers = defineDispatchers({ setCollections( _: RESTCollectionStoreType, @@ -295,18 +304,14 @@ const restCollectionDispatchers = defineDispatchers({ ) if (containingFolder === null) { - const [removed] = newState.splice(folderIndex, 1) - - newState.splice(destinationFolderIndex, 0, removed) + reorderItems(newState, folderIndex, destinationFolderIndex) return { state: newState, } } - const [removed] = containingFolder.folders.splice(folderIndex, 1) - - containingFolder.folders.splice(destinationFolderIndex, 0, removed) + reorderItems(containingFolder.folders, folderIndex, destinationFolderIndex) return { state: newState, @@ -480,9 +485,7 @@ const restCollectionDispatchers = defineDispatchers({ return {} } - const [removed] = targetLocation.requests.splice(requestIndex, 1) - - targetLocation.requests.splice(destinationRequestIndex, 0, removed) + reorderItems(targetLocation.requests, requestIndex, destinationRequestIndex) return { state: newState, diff --git a/packages/hoppscotch-common/src/newstore/reordering.ts b/packages/hoppscotch-common/src/newstore/reordering.ts new file mode 100644 index 000000000..ecf1048d4 --- /dev/null +++ b/packages/hoppscotch-common/src/newstore/reordering.ts @@ -0,0 +1,46 @@ +import { distinctUntilChanged, pluck } from "rxjs" +import DispatchingStore, { defineDispatchers } from "./DispatchingStore" + +type ReorderingItem = + | { type: "collection"; id: string; parentID: string | null } + | { type: "request"; id: string; parentID: string | null } + +type CurrentReorderingState = { + currentReorderingItem: ReorderingItem +} + +const initialState: CurrentReorderingState = { + currentReorderingItem: { + type: "collection", + id: "", + parentID: "", + }, +} + +const dispatchers = defineDispatchers({ + changeCurrentReorderStatus( + _, + { reorderItem }: { reorderItem: ReorderingItem } + ) { + return { + currentReorderingItem: reorderItem, + } + }, +}) + +export const currentReorderStore = new DispatchingStore( + initialState, + dispatchers +) + +export const currentReorderingStatus$ = currentReorderStore.subject$.pipe( + pluck("currentReorderingItem"), + distinctUntilChanged() +) + +export function changeCurrentReorderStatus(reorderItem: ReorderingItem) { + currentReorderStore.dispatch({ + dispatcher: "changeCurrentReorderStatus", + payload: { reorderItem }, + }) +}