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

Remove reference to data-testid from styling #10407

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

eliasylonen
Copy link
Contributor

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR refactors mobile table styling by removing data-testid references from styling and introducing a new isLabelHidden prop across chip components for better mobile display control.

  • Added sticky positioning and shadow effects for first three columns in packages/twenty-front/src/modules/object-record/record-table/record-table-body/components/RecordTableBody.tsx
  • Implemented mobile-aware label visibility in packages/twenty-front/src/modules/object-record/record-field/meta-types/display/components/ChipFieldDisplay.tsx based on table scroll position
  • Added isLabelHidden prop propagation through Chip component hierarchy (AvatarChip → Chip) for consistent mobile display control
  • Improved mobile table styling with proper z-index handling and transition effects for sticky columns

5 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@FelixMalfait
Copy link
Member

FelixMalfait commented Feb 23, 2025

Thank you! I think this broke the storybook test on ChipFieldDisplay as you can see in the CI

Edit: also lint test —
Screenshot 2025-02-23 at 10 01 25

@lucasbordeau
Copy link
Contributor

lucasbordeau commented Feb 24, 2025

@Bonapara Should we hide the button with the upper right arrow when we're on table on mobile ?

Because we might want to see the chip instead of the button (even if soft focus seems not really useful on mobile)

Enregistrement.de.l.ecran.2025-02-24.a.10.53.09.mov

@Bonapara
Copy link
Member

@lucasbordeau I think we could as there should not be hover behaviors on mobile.

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.

Refactor mobile table scrolling data test id
5 participants