Skip to content

Datamapper - autoscroll when connecting nodes#372

Open
VincentvanMedenbach wants to merge 8 commits intomasterfrom
datamapper-improve-user-flow
Open

Datamapper - autoscroll when connecting nodes#372
VincentvanMedenbach wants to merge 8 commits intomasterfrom
datamapper-improve-user-flow

Conversation

@VincentvanMedenbach
Copy link
Contributor

No description provided.

@VincentvanMedenbach VincentvanMedenbach changed the title Datamapper improve user flow Datamapper - autoscroll when connecting nodes Mar 23, 2026
Copy link
Contributor

@stijnpotters1 stijnpotters1 left a comment

Choose a reason for hiding this comment

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

Also try to fix the linting warnings if possible

const updatePosition = (event: MouseEvent) => {
if (scrollIntervalEnabled.current) {
const topThreshold = 150
const bottomThreshold = window.innerHeight - 150
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const bottomThreshold = window.innerHeight - 150
const bottomThreshold = window.innerHeight - topTreshold

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved the 150 to a const

if (event.clientY < topThreshold) {
reactFlowInstance.setViewport({
x: reactFlowInstance.getViewport().x,
y: reactFlowInstance.getViewport().y + 10,
Copy link
Contributor

@stijnpotters1 stijnpotters1 Mar 24, 2026

Choose a reason for hiding this comment

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

Is this because of padding?
Perhaps add a padding const to replace it with if this is true

const paddingY = 10
y: reactFlowInstance.getViewport().y + paddingY,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the amount it scrolls on the reactflow pane, moved it to a seperate const

if (event.clientY > bottomThreshold) {
reactFlowInstance.setViewport({
x: reactFlowInstance.getViewport().x,
y: reactFlowInstance.getViewport().y - 10,
Copy link
Contributor

Choose a reason for hiding this comment

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

Here as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it to a const here as well

return () => {
document.removeEventListener('mousemove', updatePosition)
}
}, [])
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you already check if you could solve these eslint warnings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the one eslint warning created by my new additions, I'm currently working on code cleanup on a different branch where i'll look at the other ones

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants