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

Fix ellipsis overflow causing edit icon to be hidden on links #5071

Merged
merged 5 commits into from
Apr 24, 2024

Conversation

DevanandGowda
Copy link
Contributor

Fixes #5064

Demo

ellipsis-overflow-fix-demo.mov

@FelixMalfait
Copy link
Member

Thanks a lot

That's the easy way to solve it.
For pills it doesn't bother me to have a max-width everywhere.
But for the email field for example it's not great as it has an impact on list views:
Screenshot 2024-04-20 at 11 09 36

I wonder if you can find something smarter to avoid hardcoding a max width.
Ideally w we should adapt when there's a buttonIcon or find another solution that doesn't affect list views.
It might be a difficult problem but it'd be great if you can try - we can probably do better!

@DevanandGowda
Copy link
Contributor Author

DevanandGowda commented Apr 20, 2024

Ahh good catch, wasn't aware the columns are extendable, so this effects all the fields including the email field.

I will see if I can try to avoid hardcoding the width while accomodating the edit button, might be possible by using something like visibility: hidden to retain the space occupied by the edit button.

Or perhaps we can implement something similar to what we do in the table, i.e have the edit button on right most edge of the field with higher z-index, although it might be annoying if the field value is small and the edit button still shows up in the right most edge.

Screenshot 2024-04-20 at 4 23 40 PM

@DevanandGowda
Copy link
Contributor Author

DevanandGowda commented Apr 20, 2024

@FelixMalfait I went with the above idea, mainly becuase I felt it's more consistent with what's present in the table view. Additionally, this also fixes the issue you caught wrt email. Here's the demo, let me know what you think. Thanks.

ellipsis-overflow-fix-demo-2.mov

Copy link
Member

@FelixMalfait FelixMalfait left a comment

Choose a reason for hiding this comment

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

Thanks!

@FelixMalfait FelixMalfait merged commit b634057 into twentyhq:main Apr 24, 2024
6 of 7 checks passed
@DevanandGowda DevanandGowda deleted the fix/ellipsis-overflow branch April 24, 2024 12:05
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.

Ellipsis doesn't take into account clickable fields
2 participants