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 Floating Input Padding and Border Color #7328

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

nganphan123
Copy link
Contributor

@nganphan123 nganphan123 commented Sep 30, 2024

This PR fix the padding and border color of floating text input #7286
The text area automatically has padding of 4px so I reset it to 0 and adjusting container padding to 8px.

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 adjusts the styling of the TextAreaInput component to address padding and border color issues, aligning with the desired behavior specified in issue #7286.

  • Modified StyledTextAreaContainer in TextAreaInput.tsx to use theme.border.color.medium for border color
  • Adjusted padding in StyledTextAreaContainer to ${({ theme }) => theme.spacing(2)} (8px)
  • Removed padding: 0px; from StyledTextArea to allow container padding to take effect
  • Adjusted width in StyledTextArea to calc(100% - ${({ theme }) => theme.spacing(7)}) for proper alignment

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

@@ -33,13 +33,14 @@ const StyledTextArea = styled(TextareaAutosize)`
resize: none;
max-height: 400px;
width: calc(100% - ${({ theme }) => theme.spacing(7)});
padding: 0px;
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Setting padding to 0px may cause text to touch the edges of the textarea. Consider using a small padding value instead.

@@ -33,13 +33,14 @@ const StyledTextArea = styled(TextareaAutosize)`
resize: none;
max-height: 400px;
width: calc(100% - ${({ theme }) => theme.spacing(7)});
Copy link
Contributor

Choose a reason for hiding this comment

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

style: The width calculation may cause overflow issues. Consider using width: 100% and adjusting padding instead.

position: relative;
width: 100%;
padding: ${({ theme }) => theme.spacing(2)} ${({ theme }) => theme.spacing(1)};
padding: ${({ theme }) => theme.spacing(2)};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
padding: ${({ theme }) => theme.spacing(2)};
padding: ${({ theme }) => theme.spacing(2)} ${({ theme }) => theme.spacing(0)};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @martmull , thank you for reviewing. I fixed according to this. I'm new to this repo, could you let me know why this would work better than the other way? Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to directly set padding to 0, instead of setting it to 1 then 0

@martmull martmull enabled auto-merge (squash) October 2, 2024 07:39
@martmull martmull merged commit 57eaa01 into twentyhq:main Oct 2, 2024
4 of 8 checks passed
harshit078 pushed a commit to harshit078/twenty that referenced this pull request Oct 14, 2024
This PR fix the padding and border color of floating text input twentyhq#7286 
The text area automatically has padding of 4px so I reset it to 0 and
adjusting container padding to 8px.
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.

2 participants