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

Focused cell is acting weird when scrolling horizontally #5595

Closed
ijreilly opened this issue May 27, 2024 · 14 comments · Fixed by #7451
Closed

Focused cell is acting weird when scrolling horizontally #5595

ijreilly opened this issue May 27, 2024 · 14 comments · Fixed by #7451
Assignees
Labels
good first issue Good for newcomers prio: med scope: front Issues that are affecting the frontend side only size: short

Comments

@ijreilly
Copy link
Collaborator

Enregistrement.de.l.ecran.2024-05-27.a.13.41.31.mov
Copy link
Contributor

greptile-apps bot commented May 27, 2024

Disclaimer: This comment was automatically generated with AI. It can give some useful hints as to where to look for, but sometimes it can also give very inaccurate answers so takes this with a pinch of salt :).

To address the issue of the focused cell acting weird when scrolling horizontally, follow these steps:

  1. Locate the relevant component: The issue is likely within the table components handling activities. Check /packages/twenty-front/src/modules/activities/table/components/CellCommentChip.tsx and /packages/twenty-front/src/modules/activities/table/components/CommentChip.tsx.

  2. Check the scrolling behavior: Ensure that the focused cell maintains its state and position correctly during horizontal scrolling. This might involve adjusting the CSS or the component's state management.

  3. Adjust the context if necessary: Review the InlineCellMutationContext and useInlineCell hook in /packages/twenty-front/src/modules/object-record/record-inline-cell to ensure the focused cell's state is managed correctly during scroll events.

  4. Modify the component: If needed, add or adjust event listeners for scroll events to maintain the focus state. Example:

useEffect(() => {
  const handleScroll = () => {
    // Logic to maintain focus state
  };
  window.addEventListener('scroll', handleScroll);
  return () => window.removeEventListener('scroll', handleScroll);
}, []);
  1. Test the changes: Ensure the focused cell behaves correctly during horizontal scrolling.

References

/packages/twenty-front/src/modules/activities/table
/packages/twenty-front/src/modules/activities/table/components
/packages/twenty-front/src/modules/object-record/record-inline-cell

Ask Greptile

@ijreilly ijreilly added the good first issue Good for newcomers label May 27, 2024
@its-id
Copy link
Contributor

its-id commented May 31, 2024

looking into it. let me see if this can be fixed.

@ijreilly
Copy link
Collaborator Author

ijreilly commented Jun 5, 2024

  • this could be linked - when selecting multiple rows they become transparent
Enregistrement.de.l.ecran.2024-06-05.a.12.23.02.mov
Capture d’écran 2024-06-05 à 12 23 57

@Nabhag8848
Copy link
Contributor

Nabhag8848 commented Aug 14, 2024

Working on it 🔥 - P0 priority.

  • Issue with when selecting multiple rows looks like is been already resolved.

Edit: changing priority to P2, this is tricky then i thought, just z-index won't fix this i believe.

also Question: Also later we will have a behaviour that when cell is focused, table shouldn't be scrollable (left and right) ideally this is what I think behaviour is, tackling both makes at the same time makes more sense to me.

@charlesBochet
Copy link
Member

Duplicated issue: #6813, let's make sure both are fixed

@charlesBochet
Copy link
Member

The issue is still there!

@charlesBochet charlesBochet added scope: front Issues that are affecting the frontend side only size: short prio: med labels Sep 14, 2024
@Bonapara
Copy link
Member

Bonapara commented Oct 2, 2024

/oss.gg 150

Copy link

oss-gg bot commented Oct 2, 2024

Thanks for opening an issue! It's live on oss.gg!

@kshitij-codes
Copy link

/assign

Copy link

oss-gg bot commented Oct 2, 2024

Assigned to @kshitij-codes! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

@kshitij-codes kshitij-codes removed their assignment Oct 2, 2024
@AshishViradiya153
Copy link
Contributor

/assign

Copy link

oss-gg bot commented Oct 4, 2024

Assigned to @AshishViradiya153! Please open a draft PR linking this issue within 48h ⚠️ If we can't detect a PR from you linking this issue in 48h, you'll be unassigned automatically 🕹️ Excited to have you ship this 🚀

@mokshith-moksh
Copy link

/assign

Copy link

oss-gg bot commented Oct 10, 2024

This issue is already assigned to another person. Please find more issues here.

lucasbordeau added a commit that referenced this issue Oct 10, 2024
Fixes #5595
- The portal has been removed, and the focused cell is now rendered
directly within the relevant div/container.
- This ensures that the cell remains correctly positioned within the
table and is properly hidden or unfocused when the user scrolls
horizontally, fixing the issue of overlap or visibility on top of other
elements.

Co-authored-by: Lucas Bordeau <[email protected]>
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in 🎯 Roadmap & Sprints Oct 10, 2024
harshit078 pushed a commit to harshit078/twenty that referenced this issue Oct 14, 2024
Fixes twentyhq#5595
- The portal has been removed, and the focused cell is now rendered
directly within the relevant div/container.
- This ensures that the cell remains correctly positioned within the
table and is properly hidden or unfocused when the user scrolls
horizontally, fixing the issue of overlap or visibility on top of other
elements.

Co-authored-by: Lucas Bordeau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers prio: med scope: front Issues that are affecting the frontend side only size: short
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants