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

Bug Fix: created new div and p tag styles and wrap it on the workspace member as container #7581

Conversation

SyedHamzaHussain000
Copy link
Contributor

Hello,
Hope you are doing well.I created a special style for the text to make sure it stays in one line and wont exceed the width if the text width will be more then 80px it will ecplise and set ... at the end of the text.

I created these 2 styles variables and wrap my text in these styles
StyledObjectSummary
StyledEllipsisParagraph

Fixes #7574

#Screens Shots
Screenshot 2024-10-10 at 10 58 04 PM

Screenshot 2024-10-10 at 10 58 20 PM

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 pull request addresses text overflow issues in workspace member displays by introducing new styled components for text truncation with ellipsis.

  • Added StyledEllipsisParagraph in SettingsDataModelObjectSummary.tsx to truncate text exceeding 80px width
  • Removed StyledObjectName and object icon display from SettingsDataModelObjectSummary.tsx
  • Modified SettingsDataModelFieldSettingsFormCard.tsx and SettingsDataModelFieldRelationSettingsFormCard.tsx to incorporate new text styling
  • Deleted .env.example files from both twenty-front and twenty-server packages, potentially impacting project setup
  • Inconsistency between PR description and actual changes in SettingsDataModelFieldRelationSettingsFormCard.tsx needs addressing

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

Copy link
Contributor

@ehconitin ehconitin left a comment

Choose a reason for hiding this comment

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

Left some comments

@charlesBochet
Copy link
Member

@SyedHamzaHussain000 still missing the twenty-server .env.example :)

@lucasbordeau lucasbordeau self-assigned this Oct 11, 2024
@SyedHamzaHussain000
Copy link
Contributor Author

@charlesBochet Sorry about that. I fixed that and push the new commit please check Thankyou

@charlesBochet
Copy link
Member

@SyedHamzaHussain000 why are we getting rid of the icon?

display: flex;
font-weight: ${({ theme }) => theme.font.weight.medium};
gap: ${({ theme }) => theme.spacing(1)};
const StyledEllipsisParagraph = styled.p`
Copy link
Member

Choose a reason for hiding this comment

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

name is not clear: I prefer the existing StyledObjectName :)

white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
margin: 0;
Copy link
Member

Choose a reason for hiding this comment

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

let's use a div and not a p, so we don't have to add margin: 0

@@ -37,10 +38,9 @@ export const SettingsDataModelObjectSummary = ({

return (
<StyledObjectSummary className={className}>
<StyledObjectName>
<ObjectIcon size={theme.icon.size.sm} stroke={theme.icon.stroke.md} />
Copy link
Member

Choose a reason for hiding this comment

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

why are we removing the icon?

@charlesBochet
Copy link
Member

Also the CIs are not passing, a priori because of lint issues

@charlesBochet
Copy link
Member

Taking over this one

@charlesBochet charlesBochet merged commit 6f5dc1c into twentyhq:main Oct 17, 2024
11 of 13 checks passed
Copy link

oss-gg bot commented Oct 17, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crop object name in New relation field Settings
4 participants