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

Adjust the line height and expand the maximum height of the plac… #7764

Merged

Conversation

abdullah5361k
Copy link
Contributor

fixes: #7757

What does this PR do?

We increased the line height from md to lg and the max height of the placeholder subtitle text from 2.4 to 2.8 to ensure that letters are no longer slightly cut off in the placeholder in Functions.

twenty-placeholder-text

How should this be tested?

  1. Log in
  2. Go to Settings
  3. Toggle "Advanced" settings
  4. Go to Functions

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 adjusts the styling of the placeholder subtitle text in the EmptyPlaceholderStyled component to address the issue of cut-off letters in the Functions section.

  • Increased line height from 'md' to 'lg' in StyledEmptySubTitle component
  • Expanded max-height from 2.4em to 2.8em in StyledEmptySubTitle component
  • These changes aim to resolve the text truncation issue in the Functions placeholder
  • The modifications affect the /packages/twenty-front/src/modules/ui/layout/animated-placeholder/components/EmptyPlaceholderStyled.tsx file
  • Users can verify the fix by checking the Functions section in the Advanced settings

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

line-height: ${({ theme }) => theme.text.lineHeight.md};
max-height: 2.4em;
line-height: ${({ theme }) => theme.text.lineHeight.lg};
max-height: 2.8em;
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider using a more flexible approach, such as max-height: calc(2 * ${({ theme }) => theme.text.lineHeight.lg}); to ensure consistency with the theme's line height.

Copy link

github-actions bot commented Oct 16, 2024

Welcome!

Hello there, congrats on your first PR! We're excited to have you contributing to this project.
By submitting your Pull Request, you acknowledge that you agree with the terms of our Contributor License Agreement.

Generated by 🚫 dangerJS against 9a4ae36

@Bonapara
Copy link
Member

Bonapara commented Oct 17, 2024

Can you make the row gap between the title and description 8px instead of 12px? (Increasing the line height enlarges the spacing between lines.)

@abdullah5361k
Copy link
Contributor Author

Can you make the row gap between the title and description 8px instead of 12px? (Increasing the line height enlarges the spacing between lines.)

Yes, I have adjusted the row gap between the title and description to 8px

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

Thank you, LGTM :)

@charlesBochet charlesBochet merged commit 2501017 into twentyhq:main Oct 21, 2024
13 checks passed
Copy link

oss-gg bot commented Oct 21, 2024

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

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.

Letters are slightly cut in placeholder in Functions
4 participants