Skip to content
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

Render only needed sortable items #1096

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
625f8d1
active and active event ok
alissaVrk Mar 29, 2023
dd88743
no re-render non dragged items on drag start / end
alissaVrk Mar 29, 2023
9241989
rename
alissaVrk Mar 29, 2023
38e3c18
remove active from reducer
alissaVrk Mar 29, 2023
ef3d7d8
add test for useDraggable for render only dragging element
alissaVrk Mar 30, 2023
ca2b3dc
do not re-render draggable and droppabale when over changes
alissaVrk Mar 30, 2023
10ec00d
remove dragging css class from droppable story, it did nothing and cr…
alissaVrk Mar 30, 2023
acdfb55
remove hasActive from useDroppable
alissaVrk Mar 30, 2023
18a7ce5
add test for render only relevant draggable element and container
alissaVrk Mar 30, 2023
618d22b
sortable renders story
alissaVrk Mar 31, 2023
83207fb
runs, not rendering all
alissaVrk Mar 31, 2023
a873505
some fixes
alissaVrk Apr 1, 2023
cd2d0d4
calculate hasActive in useSortable
alissaVrk Apr 1, 2023
b356522
fix default context for drag overlay
alissaVrk Apr 1, 2023
3f15507
Merge branch 'render-only-dragging-item' into render-only-over-item
alissaVrk Apr 1, 2023
950e42e
Merge branch 'render-only-over-item' into render-only-needed-sortable…
alissaVrk Apr 1, 2023
1affa4f
can drag over in sortable without re-render all. some small issues to…
alissaVrk Apr 1, 2023
95199cb
clean up
alissaVrk Apr 1, 2023
503adb8
return strategy delta for active as well
alissaVrk Apr 2, 2023
1ab04f7
do not animate drop by default
alissaVrk Apr 2, 2023
6dbc5f8
pass over from droppable as well (more useful)
alissaVrk Apr 3, 2023
33bf5e1
fix pages story
alissaVrk Apr 3, 2023
67e219c
fix drawer example
alissaVrk Apr 3, 2023
d2d5043
fix type in switch
alissaVrk Apr 3, 2023
ab54c30
Merge branch 'render-only-dragging-item' into render-only-needed-sort…
alissaVrk Apr 3, 2023
f79a6e1
types and styling in stories
alissaVrk Apr 3, 2023
dc114ec
add active for over item
alissaVrk Apr 4, 2023
1c7be36
cache prev items and container in item and expose global active ref i…
alissaVrk Apr 4, 2023
90d0baf
add useConditionalDndContext
alissaVrk Apr 4, 2023
a19a3e2
add test for render only needed sortable items
alissaVrk Apr 4, 2023
d93b2af
fix strategy delta diffs and pass disable transforms only for active …
alissaVrk Apr 5, 2023
baf7e7e
add tests for drop + memo items in test
alissaVrk Apr 5, 2023
38cd6e9
remove only
alissaVrk Apr 5, 2023
64a4c7a
uncomment items dependency
alissaVrk Apr 25, 2023
358d37d
fix typo
alissaVrk Aug 4, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions cypress/integration/draggable_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,4 +416,26 @@ describe('Draggable', () => {
});
});
});

describe('Multiple Draggables', () => {
it('should render only dragging element', () => {
cy.visitStory('core-draggable-draggablerenders--basic-setup')
.findFirstDraggableItem()

.mouseMoveBy(0, 100);

cy.get('[data-testid="draggable-status-1"]').should(
'have.text',
'updated'
);
cy.get('[data-testid="draggable-status-2"]').should(
'have.text',
'mounted'
);
cy.get('[data-testid="draggable-status-3"]').should(
'have.text',
'mounted'
);
});
});
});
100 changes: 100 additions & 0 deletions cypress/integration/droppable_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/// <reference path="../support/index.d.ts" />

describe('Droppable', () => {
describe('Droppable Renders', () => {
it('should re-render only the dragged item and the container dragged over - no drop', () => {
cy.visitStory('core-droppablerenders-usedroppable--multiple-droppables');

cy.get('[data-cypress="droppable-container-A"]').then((droppable) => {
const coords = droppable[0].getBoundingClientRect();
return cy
.get('[data-cypress="draggable-item"]')
.first()
.then((draggable) => {
const initialCoords = draggable[0].getBoundingClientRect();
return cy
.wrap(draggable, {log: false})
.mouseMoveBy(
coords.x - initialCoords.x + 10,
coords.y - initialCoords.y + 10,
{
delay: 1000,
noDrop: true,
}
);
});
});

cy.get('[data-testid="draggable-status-1"]').should(
'have.text',
'updated'
);
cy.get('[data-testid="draggable-status-2"]').should(
'have.text',
'mounted'
);
cy.get('[data-testid="draggable-status-3"]').should(
'have.text',
'mounted'
);

cy.get('[data-testid="droppable-status-A"]').should(
'have.text',
'updated'
);
cy.get('[data-testid="droppable-status-B"]').should(
'have.text',
'mounted'
);
cy.get('[data-testid="droppable-status-C"]').should(
'have.text',
'mounted'
);
});

it('should re-render only the dragged item and the container dragged over - with drop', () => {
cy.visitStory('core-droppablerenders-usedroppable--multiple-droppables');

cy.get('[data-cypress="droppable-container-A"]').then((droppable) => {
const coords = droppable[0].getBoundingClientRect();
return cy
.get('[data-cypress="draggable-item"]')
.last()
.then((draggable) => {
const initialCoords = draggable[0].getBoundingClientRect();
return cy
.wrap(draggable, {log: false})
.mouseMoveBy(
coords.x - initialCoords.x + 10,
coords.y - initialCoords.y + 10,
{
delay: 1000,
noDrop: false,
}
);
});
});

//the dropped item is mounted because it moves to a different container
for (let i = 1; i <= 3; i++) {
cy.get(`[data-testid="draggable-status-${i}"]`).should(
'have.text',
'mounted'
);
}

cy.get('[data-testid="droppable-status-A"]').should(
'have.text',
'updated'
);
cy.get('[data-testid="droppable-status-B"]').should(
'have.text',
'mounted'
);
cy.get('[data-testid="droppable-status-C"]').should(
'have.text',
'mounted'
);
});
});
});
49 changes: 49 additions & 0 deletions cypress/integration/sortable_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,3 +543,52 @@ describe('Sortable Virtualized List', () => {
});
});
});

describe('Sortable Renders only what is necessary ', () => {
it('should render active and items between active and over - no drop', () => {
cy.visitStory('presets-sortable-renders--basic-setup');

cy.get('[data-cypress="draggable-item"]').then((droppables) => {
const coords = droppables[1].getBoundingClientRect(); //drop after item id - 3
return cy
.findFirstDraggableItem()
.mouseMoveBy(coords.x + 10, coords.y + 10, {delay: 1, noDrop: true});
});

for (let id = 1; id <= 3; id++) {
cy.get(`[data-testid="sortable-status-${id}"]`).should(
'have.text',
`updated ${id}`
);
}

for (let id = 4; id <= 10; id++) {
cy.get(`[data-testid="sortable-status-${id}"]`).should(
'have.text',
`mounted ${id}`
);
}
});

//we test for drop in place, because otherwise items change and cause a real re-render to all items
//probably possible to fix that too but I didn't get there
it('should render active only on d&d in place - with drop', () => {
cy.visitStory('presets-sortable-renders--basic-setup');

cy.findFirstDraggableItem().mouseMoveBy(10, 10, {
delay: 1,
noDrop: false,
});

cy.get(`[data-testid="sortable-status-1"]`).should(
'have.text',
`updated 1`
);
for (let id = 2; id <= 10; id++) {
cy.get(`[data-testid="sortable-status-${id}"]`).should(
'have.text',
`mounted ${id}`
);
}
});
});
58 changes: 33 additions & 25 deletions cypress/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function getDocumentScroll() {
}

Cypress.Commands.add('findFirstDraggableItem', () => {
return cy.get(`[data-cypress="draggable-item"`);
return cy.get(`[data-cypress="draggable-item"]`).first();
});

Cypress.Commands.add(
Expand All @@ -47,7 +47,12 @@ Cypress.Commands.add(
{
prevSubject: 'element',
},
(subject, x: number, y: number, options?: {delay: number}) => {
(
subject,
x: number,
y: number,
options?: {delay: number; noDrop?: boolean}
) => {
cy.wrap(subject, {log: false})
.then((subject) => {
const initialRect = subject.get(0).getBoundingClientRect();
Expand All @@ -56,7 +61,8 @@ Cypress.Commands.add(
return [subject, initialRect, windowScroll] as const;
})
.then(([subject, initialRect, initialWindowScroll]) => {
cy.wrap(subject)
let resultOps = cy
.wrap(subject)
.trigger('mousedown', {force: true})
.wait(options?.delay || 0, {log: Boolean(options?.delay)})
.trigger('mousemove', {
Expand All @@ -72,29 +78,31 @@ Cypress.Commands.add(
force: true,
clientX: Math.floor(initialRect.left + initialRect.width / 2 + x),
clientY: Math.floor(initialRect.top + initialRect.height / 2 + y),
})
.wait(100)
.trigger('mouseup', {force: true})
.wait(250)
.then((subject: any) => {
const finalRect = subject.get(0).getBoundingClientRect();
const windowScroll = getDocumentScroll();
const windowScrollDelta = {
x: windowScroll.x - initialWindowScroll.x,
y: windowScroll.y - initialWindowScroll.y,
};

const delta = {
x: Math.round(
finalRect.left - initialRect.left - windowScrollDelta.x
),
y: Math.round(
finalRect.top - initialRect.top - windowScrollDelta.y
),
};

return [subject, {initialRect, finalRect, delta}] as const;
});

if (!options?.noDrop) {
resultOps = resultOps.wait(100).trigger('mouseup', {force: true});
}

resultOps.wait(250).then((subject: any) => {
const finalRect = subject.get(0).getBoundingClientRect();
const windowScroll = getDocumentScroll();
const windowScrollDelta = {
x: windowScroll.x - initialWindowScroll.x,
y: windowScroll.y - initialWindowScroll.y,
};

const delta = {
x: Math.round(
finalRect.left - initialRect.left - windowScrollDelta.x
),
y: Math.round(
finalRect.top - initialRect.top - windowScrollDelta.y
),
};

return [subject, {initialRect, finalRect, delta}] as const;
});
});
}
);
Expand Down
2 changes: 1 addition & 1 deletion cypress/support/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ declare namespace Cypress {
mouseMoveBy(
x: number,
y: number,
options?: {delay: number}
options?: {delay: number; noDrop?: boolean}
): Chainable<
[
Element,
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"@types/classnames": "^2.2.11",
"@types/react": "^16.9.43",
"@types/react-dom": "^16.9.8",
"@types/use-sync-external-store": "^0.0.3",
"babel-jest": "^27.0.2",
"babel-loader": "^8.2.1",
"chromatic": "^5.4.0",
Expand Down
5 changes: 3 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@
"react-dom": ">=16.8.0"
},
"dependencies": {
"tslib": "^2.0.0",
"@dnd-kit/accessibility": "^3.0.0",
"@dnd-kit/utilities": "^3.2.1"
"@dnd-kit/utilities": "^3.2.1",
"tslib": "^2.0.0",
"use-sync-external-store": "^1.2.0"
},
"publishConfig": {
"access": "public"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ interface Props {
}

export function RestoreFocus({disabled}: Props) {
const {active, activatorEvent, draggableNodes} = useContext(InternalContext);
const {useGloablActive, useGlobalActivatorEvent, draggableNodes} =

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, just a typo here (should be useGlobalActive)

useContext(InternalContext);
const active = useGloablActive();
const activatorEvent = useGlobalActivatorEvent();

const previousActivatorEvent = usePrevious(activatorEvent);
const previousActiveId = usePrevious(active?.id);

Expand Down
Loading