-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fix: Blocklist table optimised for all viewports #7618
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This pull request optimizes the blocklist table in the settings for all viewports, addressing the overflow issue with long email addresses.
- Modified
SettingsAccountsBlocklistTable.tsx
to add responsive styles, particularly for mobile viewports - Added right alignment to the 'Added to blocklist' column for better layout
- Set max-width for table body on mobile screens to prevent overflow
- Improved overall table layout and responsiveness for long email addresses
- Addressed issue Blocklist table overflows in long emails #7549 by preventing long emails from affecting other columns
1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
...ges/twenty-front/src/modules/settings/accounts/components/SettingsAccountsBlocklistTable.tsx
Outdated
Show resolved
Hide resolved
@media (max-width: ${MOBILE_VIEWPORT}px) { | ||
max-width: 60%; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: max-width: 60% might cause content to be cut off on very small screens
margin-left: ${({ theme }) => theme.spacing(6)}; | ||
} | ||
|
||
@media (min-width: 360px) and (max-width: 390px) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have these 360 and 390px limit been discussed with design
As far as I know we only have the MOBILE_VIEWPORT limit on the project right now.
I'm not against introducing behaviors for 360px but we need to have a project-wide approach in this case.
Can we revert this change and get the input of @Bonapara on the overall approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@charlesBochet sure, I'll revert the min width code blocks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@harshit078 I have found a simpler way to achieve a nice result
/award 150 |
Awarding harshit078: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/harshit078 |
Thanks @harshit078 for your contribution! |
Description
Changes -
Screen.Recording.2024-10-12.at.5.12.50.PM.mov