Improve board experience 🖼️ (#16063)
This PR improves the general UX and DX of boards, by modifying the query effect to only use paged group by queries. In this PR we implement two more things in the backend for group by queries : - Fixed ORDER BY in the PARTITION BY sub-query (this wasn't working because it was applied in the main query, so it sorted randomly picked records, which was a correct sort on an incorrect dataset returned by the sub-query) - Added offset paging in PARTITION BY Miscellaneous, various bug fixes and improvements along the way : - Throttled loading of cards to avoid React freeze - Handling of drag & drop - Handling of create / delete / update - Reworked skeleton (the library slows down a lot with hundreds of skeleton for a spinning effect that is hardly noticed) - Fixed refetch of aggregate queries (I included the new group by aggregates query we use in the existing refetch mechanism) - Re-trigger queries on filters and sorts changes - Unselect all record ids when deleting / restoring / detroying - Fetch only groups that still have records to lighten the group by query. # What remains to be done This is still a naïve fetch more implementation that will work for a few fetch more rounds, but if you scroll and load say 200 cards per column on a board, React will re-render all 200 cards of each column each time. We would probably need to virtualize the board with paged queries as we did for the table, this could be done after this PR but seems less urgent. What's nice is that this new query pattern is well designed for virtualization also, drawing from our experience with table virtualization, and adapted to a multi-column request pattern, like a 2:2 matrix of records, for our boards. So the remaining work would be to design a UI solution for virtualizing this matrix of records, which could be quite different from our table virtualization mechanism.
This commit is contained in:
+3
-3
@@ -2,7 +2,7 @@
|
||||
|
||||
exports[`generateGroupByQuery should generate valid GraphQL query for empty aggregate operations 1`] = `
|
||||
"
|
||||
query PeopleGroupBy(
|
||||
query PeopleGroupByAggregates(
|
||||
$groupBy: [PersonGroupByInput!]!
|
||||
$filter: PersonFilterInput
|
||||
$orderBy: [PersonOrderByWithGroupByInput!]
|
||||
@@ -24,7 +24,7 @@ exports[`generateGroupByQuery should generate valid GraphQL query for empty aggr
|
||||
|
||||
exports[`generateGroupByQuery should generate valid GraphQL query for multiple aggregate operations 1`] = `
|
||||
"
|
||||
query OpportunitiesGroupBy(
|
||||
query OpportunitiesGroupByAggregates(
|
||||
$groupBy: [OpportunityGroupByInput!]!
|
||||
$filter: OpportunityFilterInput
|
||||
$orderBy: [OpportunityOrderByWithGroupByInput!]
|
||||
@@ -49,7 +49,7 @@ exports[`generateGroupByQuery should generate valid GraphQL query for multiple a
|
||||
|
||||
exports[`generateGroupByQuery should generate valid GraphQL query for single aggregate operation 1`] = `
|
||||
"
|
||||
query OpportunitiesGroupBy(
|
||||
query OpportunitiesGroupByAggregates(
|
||||
$groupBy: [OpportunityGroupByInput!]!
|
||||
$filter: OpportunityFilterInput
|
||||
$orderBy: [OpportunityOrderByWithGroupByInput!]
|
||||
|
||||
+89
-26
@@ -23,43 +23,106 @@ const mockRecordsWithPosition: RecordWithPosition[] = [
|
||||
];
|
||||
|
||||
describe('computeNewPositionOfRecordWithPosition', () => {
|
||||
it('should compute first position', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'B',
|
||||
idOfTargetItem: 'A',
|
||||
describe('dragging inside same list', () => {
|
||||
it('should compute first position', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'B',
|
||||
idOfTargetItem: 'A',
|
||||
isDroppedAfterList: false,
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(-1);
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(-1);
|
||||
it('should compute last position', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'B',
|
||||
idOfTargetItem: 'D',
|
||||
isDroppedAfterList: true,
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(4);
|
||||
});
|
||||
|
||||
it('should compute intermediary position after target item', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'A',
|
||||
idOfTargetItem: 'B',
|
||||
isDroppedAfterList: false,
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(1.5);
|
||||
});
|
||||
|
||||
it('should compute intermediary position before target item', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'A',
|
||||
idOfTargetItem: 'C',
|
||||
isDroppedAfterList: false,
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(2.5);
|
||||
});
|
||||
});
|
||||
|
||||
it('should compute last position', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'B',
|
||||
idOfTargetItem: 'D',
|
||||
describe('dragging from another list', () => {
|
||||
it('should compute first position', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'Unknown',
|
||||
idOfTargetItem: 'A',
|
||||
isDroppedAfterList: false,
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(-1);
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(4);
|
||||
});
|
||||
it('should compute last position', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'Unknown',
|
||||
idOfTargetItem: 'D',
|
||||
isDroppedAfterList: true,
|
||||
});
|
||||
|
||||
it('should compute intermediary position after target item', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'A',
|
||||
idOfTargetItem: 'B',
|
||||
expect(newPosition).toEqual(4);
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(1.5);
|
||||
});
|
||||
it('should compute position before last item', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'Unknown',
|
||||
idOfTargetItem: 'D',
|
||||
isDroppedAfterList: false,
|
||||
});
|
||||
|
||||
it('should compute intermediary position before target item', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'A',
|
||||
idOfTargetItem: 'C',
|
||||
expect(newPosition).toEqual(2.5);
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(2.5);
|
||||
it('should compute intermediary position after target item', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'Unknown',
|
||||
idOfTargetItem: 'B',
|
||||
isDroppedAfterList: false,
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(0.5);
|
||||
});
|
||||
|
||||
it('should compute intermediary position before target item', () => {
|
||||
const newPosition = computeNewPositionOfDraggedRecord({
|
||||
arrayOfRecordsWithPosition: mockRecordsWithPosition,
|
||||
idOfItemToMove: 'Unknown',
|
||||
idOfTargetItem: 'C',
|
||||
isDroppedAfterList: false,
|
||||
});
|
||||
|
||||
expect(newPosition).toEqual(1.5);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+6
-6
@@ -10,10 +10,12 @@ export const computeNewPositionOfDraggedRecord = ({
|
||||
arrayOfRecordsWithPosition,
|
||||
idOfItemToMove,
|
||||
idOfTargetItem,
|
||||
isDroppedAfterList,
|
||||
}: {
|
||||
arrayOfRecordsWithPosition: RecordWithPosition[];
|
||||
idOfItemToMove: string;
|
||||
idOfTargetItem: string;
|
||||
isDroppedAfterList: boolean;
|
||||
}) => {
|
||||
const targetItem = arrayOfRecordsWithPosition.find(
|
||||
(recordToFind) => recordToFind.id === idOfTargetItem,
|
||||
@@ -29,6 +31,10 @@ export const computeNewPositionOfDraggedRecord = ({
|
||||
|
||||
const targetPosition = targetItem.position;
|
||||
|
||||
if (isDroppedAfterList) {
|
||||
return targetPosition + 1;
|
||||
}
|
||||
|
||||
const sortedRecordsByAscendingPosition = arrayOfRecordsWithPosition.toSorted(
|
||||
sortByProperty('position'),
|
||||
);
|
||||
@@ -43,16 +49,10 @@ export const computeNewPositionOfDraggedRecord = ({
|
||||
(recordToFind) => recordToFind.id === idOfTargetItem,
|
||||
);
|
||||
|
||||
const lastIndex = sortedRecordsByAscendingPosition.length - 1;
|
||||
|
||||
const shouldGoToFirstPosition = indexOfTargetItem === 0;
|
||||
|
||||
const shouldGoToLastPosition = indexOfTargetItem === lastIndex;
|
||||
|
||||
if (shouldGoToFirstPosition) {
|
||||
return targetPosition - 1;
|
||||
} else if (shouldGoToLastPosition) {
|
||||
return targetPosition + 1;
|
||||
} else {
|
||||
if (itemToMoveIsNotInTable) {
|
||||
const itemBeforeTargetItem =
|
||||
|
||||
+3
-5
@@ -8,11 +8,13 @@ export const computeNewPositionsOfDraggedRecords = ({
|
||||
draggedRecordId,
|
||||
targetRecordId,
|
||||
sourceRecordIds,
|
||||
isDroppedAfterList,
|
||||
}: {
|
||||
arrayOfRecordsWithPosition: RecordWithPosition[];
|
||||
draggedRecordId: string;
|
||||
targetRecordId: string;
|
||||
sourceRecordIds: string[];
|
||||
isDroppedAfterList: boolean;
|
||||
}): RecordWithPosition[] | null => {
|
||||
const targetItem = arrayOfRecordsWithPosition.find(
|
||||
(recordToFind) => recordToFind.id === targetRecordId,
|
||||
@@ -38,12 +40,8 @@ export const computeNewPositionsOfDraggedRecords = ({
|
||||
(recordToFind) => recordToFind.id === targetRecordId,
|
||||
);
|
||||
|
||||
const lastIndex = arrayOfRecordsWithPosition.length - 1;
|
||||
|
||||
const shouldGoToFirstPosition = indexOfTargetItem === 0;
|
||||
|
||||
const shouldGoToLastPosition = indexOfTargetItem === lastIndex;
|
||||
|
||||
if (shouldGoToFirstPosition) {
|
||||
const newPositions = computeNewEvenlySpacedPositions({
|
||||
startingPosition: targetPosition - 1,
|
||||
@@ -58,7 +56,7 @@ export const computeNewPositionsOfDraggedRecords = ({
|
||||
}));
|
||||
|
||||
return newSourceRecordsWithPosition;
|
||||
} else if (shouldGoToLastPosition) {
|
||||
} else if (isDroppedAfterList) {
|
||||
const newPositions = computeNewEvenlySpacedPositions({
|
||||
startingPosition: targetPosition,
|
||||
endingPosition: targetPosition + sourceRecordIds.length + 1,
|
||||
|
||||
Reference in New Issue
Block a user