fix: Minor bug in column scroll in mobile viewport #7448

harshit078 · 2024-10-05T06:58:54Z

Note

This PR fixes Minor bug in column scroll in mobile viewport #7447

greptile-apps

PR Summary

This pull request addresses a minor bug in column scrolling for mobile viewports. The change reduces the width of a specific table header cell from 35px to 30px.

Modified packages/twenty-front/src/modules/object-record/record-table/record-table-header/components/RecordTableHeader.tsx to adjust cell width
Reduced width of third table header cell from 35px to 30px for mobile viewports
Aims to prevent first letter of input from being displayed during column scrolling on mobile
Further testing on mobile devices recommended to ensure full resolution of the issue

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

charlesBochet · 2024-10-05T08:31:43Z

@harshit078 Thanks for the issue AND the PR

I have some feedbacks / questions:

Regarding the issue on the RecordTable, your fix works but the ideal fix would be to display the first column chip itself in a "compact mode" where we only see the icon. I would add a prop on top of the AvatarChip (isCompact), add a property to the FieldContext isCompactDisplay and set it to true in the case of isMobile
Regarding the other issue on the RightDrawer, I managed to reproduce it but I'm not sure to understand how it's happening (looks like the margin is still applied and mess with the design) but I'm not sure your fix is correct as it might break other behaviors, I'm double checking

charlesBochet · 2024-10-05T08:36:04Z

...y-front/src/modules/object-record/record-inline-cell/components/RecordInlineCellEditMode.tsx

@@ -11,7 +11,7 @@ const StyledInlineCellEditModeContainer = styled.div`
  display: flex;
  height: 24px;

-  margin-left: -${({ theme }) => theme.spacing(1)};
+  margin-left: ${({ theme }) => theme.spacing(1)};


changing it might break the alignment for other inline edit input

charlesBochet · 2024-10-05T08:59:18Z

Regarding my second point, it's indeed breaking the alignment of the inplace input:

(the placeholder of the edit input should be in the exact same spot as the display text)

I think there is something very wrong with the current implementation, I'm making a proposal in your PR

charlesBochet · 2024-10-05T09:24:43Z

packages/twenty-front/src/loading/components/__stories__/PrefetchLoading.stories.tsx

@@ -38,6 +38,6 @@ export const Default: Story = {
    await canvas.findByText('Tasks');
    await canvas.findByText('People');
    await canvas.findByText('Opportunities');
-    await canvas.findByText('My Customs');
+    await canvas.findByText('Rockets');


Fixing an unrelated story

charlesBochet · 2024-10-05T09:24:56Z

...es/twenty-front/src/modules/object-record/record-inline-cell/components/RecordInlineCell.tsx

@@ -24,15 +24,13 @@ import {
 type RecordInlineCellProps = {
  readonly?: boolean;
  loading?: boolean;
-  isCentered?: boolean;


no need to drilldown props as we already have it from the context

charlesBochet · 2024-10-05T09:25:16Z

...y-front/src/modules/object-record/record-inline-cell/components/RecordInlineCellEditMode.tsx

 const StyledInlineCellEditModeContainer = styled.div`
  align-items: center;

  display: flex;
+  width: 100%;


this is correct and ease positioning

charlesBochet · 2024-10-05T09:25:26Z

...y-front/src/modules/object-record/record-inline-cell/components/RecordInlineCellEditMode.tsx

    middleware: [
      flip(),
      offset(
        isCentered
          ? {
-              mainAxis: -32,


this was non sense :p

charlesBochet · 2024-10-05T09:28:30Z

/award 150

oss-gg · 2024-10-05T09:28:33Z

Awarding harshit078: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/harshit078

charlesBochet · 2024-10-05T09:30:25Z

...lineActivities/rows/main-object/components/__stories__/EventRowMainObjectUpdated.stories.tsx

@@ -36,7 +36,7 @@ const meta: Meta<typeof EventRowMainObjectUpdated> = {
      },
    } as TimelineActivity,
    mainObjectMetadataItem: generatedMockObjectMetadataItems.find(
-      (item) => item.namePlural === 'person',
+      (item) => item.nameSingular === 'person',


non related too

charlesBochet

I will actually approve it as it looks not so bad regarding the table too! I still think we could implement something more robust but it still a good step forward

Thank you @harshit078 :)

github-actions · 2024-10-05T09:47:42Z

Thanks @harshit078 for your contribution!
This marks your 23rd PR on the repo. You're top 2% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

harshit078 · 2024-10-05T09:50:40Z

@charlesBochet The whole misalignment of RecordInline cell container started from #6899 where it was decided to reduce the width of container which led to their current looking state.

harshit078 · 2024-10-05T09:54:27Z

One more feature to work on would be to improve the overall snap and scroll behaviour for ismobile when scrolling across the table.

Screen.Recording.2024-10-05.at.3.24.01.PM.mov

charlesBochet · 2024-10-05T10:10:10Z

@charlesBochet The whole misalignment of RecordInline cell container started from #6899 where it was decided to reduce the width of container which led to their current looking state.

Yes but the issue was that the editModeContainer was a 0px container putted right before the displayMode which made very hard to properly align the floating inplace input. I have changed it to be absolutely positioned on top which give us more control!

Yes the we can do better regarding the snap, maybe some animation?

harshit078 · 2024-10-05T10:38:56Z

Yes, I agree to your point on giving position:absolute to gain more control on cell container. Could we do transition- ease-in-out, scroll-behavior- smooth or maybe something in the same direction. Whats your take @charlesBochet ?

> [!Note] > - This PR fixes twentyhq#7447 --------- Co-authored-by: Charles Bochet <[email protected]>

improved the chip container

1590706

greptile-apps bot reviewed Oct 5, 2024

View reviewed changes

fix

8ec8f68

charlesBochet added the -PR: awaiting review label Oct 5, 2024

charlesBochet self-assigned this Oct 5, 2024

charlesBochet added -PR: awaiting author and removed -PR: awaiting review labels Oct 5, 2024

charlesBochet reviewed Oct 5, 2024

View reviewed changes

charlesBochet added 2 commits October 5, 2024 11:22

Fix inline cell edit more positioning

00321c0

Merge branch 'main' into column-fix

4f69716

charlesBochet reviewed Oct 5, 2024

View reviewed changes

Fix another test

47f74f7

charlesBochet reviewed Oct 5, 2024

View reviewed changes

charlesBochet approved these changes Oct 5, 2024

View reviewed changes

charlesBochet merged commit 967e04f into twentyhq:main Oct 5, 2024
11 checks passed

harshit078 added a commit to harshit078/twenty that referenced this pull request Oct 14, 2024

fix: Minor bug in column scroll in mobile viewport (twentyhq#7448)

d6bc244

> [!Note] > - This PR fixes twentyhq#7447 --------- Co-authored-by: Charles Bochet <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Minor bug in column scroll in mobile viewport #7448

fix: Minor bug in column scroll in mobile viewport #7448

harshit078 commented Oct 5, 2024

greptile-apps bot left a comment

charlesBochet commented Oct 5, 2024

charlesBochet Oct 5, 2024

charlesBochet commented Oct 5, 2024

charlesBochet Oct 5, 2024

charlesBochet Oct 5, 2024

charlesBochet Oct 5, 2024

charlesBochet Oct 5, 2024

charlesBochet commented Oct 5, 2024

oss-gg bot commented Oct 5, 2024

charlesBochet Oct 5, 2024

charlesBochet left a comment

github-actions bot commented Oct 5, 2024

harshit078 commented Oct 5, 2024

harshit078 commented Oct 5, 2024

charlesBochet commented Oct 5, 2024

harshit078 commented Oct 5, 2024

fix: Minor bug in column scroll in mobile viewport #7448

fix: Minor bug in column scroll in mobile viewport #7448

Conversation

harshit078 commented Oct 5, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

charlesBochet commented Oct 5, 2024

charlesBochet Oct 5, 2024

Choose a reason for hiding this comment

charlesBochet commented Oct 5, 2024

charlesBochet Oct 5, 2024

Choose a reason for hiding this comment

charlesBochet Oct 5, 2024

Choose a reason for hiding this comment

charlesBochet Oct 5, 2024

Choose a reason for hiding this comment

charlesBochet Oct 5, 2024

Choose a reason for hiding this comment

charlesBochet commented Oct 5, 2024

oss-gg bot commented Oct 5, 2024

charlesBochet Oct 5, 2024

Choose a reason for hiding this comment

charlesBochet left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 5, 2024

harshit078 commented Oct 5, 2024

harshit078 commented Oct 5, 2024

charlesBochet commented Oct 5, 2024

harshit078 commented Oct 5, 2024