Fixed: Drag and Drop Causes Flashing and Disappearing Cards

[rksingh2001]

Issue was "inView" was setting false due to root being scrollWrapper, instead changed it to viewport.
Fixes: #6001
Screencast from 2024-06-27 22-50-59.webm

[greptile-apps]

PR Summary

• Modified useInView hook to use viewport as root instead of scrollWrapper
• Fixed issue causing cards to flash and disappear during drag and drop

_{1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings}

[charlesBochet]

Hi @rksingh2001, thanks for your PR :) I'm not sure about this fix. I would expect an optimistic rendering issue (there is a concurrency issue between the queries behind each column / the cache is not properly updated).

Let's have a closer look to your fix tomorrow

[rksingh2001]

Hi @rksingh2001, thanks for your PR :) I'm not sure about this fix. I would expect an optimistic rendering issue (there is a concurrency issue between the queries behind each column / the cache is not properly updated).

Let's have a closer look to your fix tomorrow

Sure, @charlesBochet, its still optimistic rendering, its just that the root is now viewport. Although I don't full understand this point -> (there is a concurrency issue between the queries behind each column / the cache is not properly updated).

[lucasbordeau]

Hi @rksingh2001, could you please look into the logic that updates a board card position in DB, because the problem is that now that we have one Apollo request per column, there's something that is not working with optimistic rendering in Apollo cache.

Here it shouldn't take so long to update a board card visually (but it's ok if the request is slow) and should be instantaneous, with the help of optimistic rendering.

Also we shouldn't try to fix the content of the card disappearing in this issue as it is not our concern here.

[rksingh2001]

@lucasbordeau "there's something that is not working with optimistic rendering in Apollo cache", can you be a little more specific on what exactly is not working, so I understand better.

"Also we shouldn't try to fix the content of the card disappearing in this issue as it is not our concern here." <- I don't understand this part, do we not need to fix #6001? Because then there is no need of this PR.

[rksingh2001]

I think the label is wrong, I have responded

[lucasbordeau]

@rksingh2001 This issue is quite difficult, sorry for the response delay, I'll help you with that soon !

[github-actions]

	Fails
🚫	node failed.

Log

�[31mError: �[39m RequestError [HttpError]: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 88CF:1137D6:CBEBA4D:CDABAF4:66BA43EA.
    at /home/runner/work/twenty/twenty/node_modules/�[4m@octokit�[24m/request/dist-node/index.js:86:21
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m {
  status: �[33m403�[39m,
  response: {
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3Arksingh2001%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    status: �[33m403�[39m,
    headers: {
      �[32m'access-control-allow-origin'�[39m: �[32m'*'�[39m,
      �[32m'access-control-expose-headers'�[39m: �[32m'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'�[39m,
      connection: �[32m'close'�[39m,
      �[32m'content-encoding'�[39m: �[32m'gzip'�[39m,
      �[32m'content-security-policy'�[39m: �[32m"default-src 'none'"�[39m,
      �[32m'content-type'�[39m: �[32m'application/json; charset=utf-8'�[39m,
      date: �[32m'Mon, 12 Aug 2024 17:18:34 GMT'�[39m,
      �[32m'referrer-policy'�[39m: �[32m'origin-when-cross-origin, strict-origin-when-cross-origin'�[39m,
      server: �[32m'github.com'�[39m,
      �[32m'strict-transport-security'�[39m: �[32m'max-age=31536000; includeSubdomains; preload'�[39m,
      �[32m'transfer-encoding'�[39m: �[32m'chunked'�[39m,
      vary: �[32m'Accept-Encoding, Accept, X-Requested-With'�[39m,
      �[32m'x-content-type-options'�[39m: �[32m'nosniff'�[39m,
      �[32m'x-frame-options'�[39m: �[32m'deny'�[39m,
      �[32m'x-github-api-version-selected'�[39m: �[32m'2022-11-28'�[39m,
      �[32m'x-github-media-type'�[39m: �[32m'github.v3; format=json'�[39m,
      �[32m'x-github-request-id'�[39m: �[32m'88CF:1137D6:CBEBA4D:CDABAF4:66BA43EA'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1723483174'�[39m,
      �[32m'x-ratelimit-resource'�[39m: �[32m'search'�[39m,
      �[32m'x-ratelimit-used'�[39m: �[32m'1'�[39m,
      �[32m'x-xss-protection'�[39m: �[32m'0'�[39m
    },
    data: {
      documentation_url: �[32m'https://docs.github.com/free-pro-team@latest/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits'�[39m,
      message: �[32m'You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 88CF:1137D6:CBEBA4D:CDABAF4:66BA43EA.'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3Arksingh2001%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    headers: {
      accept: �[32m'application/vnd.github.v3+json'�[39m,
      �[32m'user-agent'�[39m: �[32m'octokit-rest.js/18.12.0 octokit-core.js/3.6.0 Node.js/18.20.4 (linux; x64)'�[39m,
      authorization: �[32m'token [REDACTED]'�[39m
    },
    request: { hook: �[36m[Function: bound bound register]�[39m }
  }
}
danger-results://tmp/danger-results-2d98e777.json

Generated by 🚫 dangerJS against 411ac52

[lucasbordeau]

@charlesBochet @rksingh2001 This was a tricky bug due to a refetch caused by DELETE directive from Apollo.

I removed the handling of limit (first and last) in the optimistic rendering because it's not required to have an already working optimistic effect in a list, and we can rely on the fetch many return to overwrite incorrect optimistic effect results.

I think my big TODO comment was relevant before the many refactors to have optimistic effect working with fetch more, but since it is now stable, I think we can just remove this entire comment and this part concerning limit also.

It would be difficult to implement an optimistic effect on limit, maybe we want to do it later ? I let you decide @charlesBochet.