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

Fixing last column width in table-view #7258

Merged
merged 2 commits into from
Sep 25, 2024
Merged

Conversation

sid0-0
Copy link
Contributor

@sid0-0 sid0-0 commented Sep 25, 2024

fixes: #7160

table-layout: fixed requires a width, added that.

Screenshots:
image
image

Small screen:

image image

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 issues with the last column width in the table view, focusing on improving layout and responsiveness.

  • Modified StyledTable in RecordTable.tsx to set width to 100%, fixing layout issues
  • Reduced width and max-width of table cells on mobile from 35px to 32px in RecordTableTd.tsx
  • Adjusted styling of the last column in RecordTableHeaderLastColumn.tsx, setting fixed width to 32px and removing left padding
  • Improved table responsiveness when wider than the screen, particularly affecting the last column

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

Comment on lines 33 to 38
${({ isTableWiderThanScreen, theme }) =>
isTableWiderThanScreen
? `
width: 32px;
background-color: ${theme.background.primary};
`
: ''};
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 removing this conditional styling as it's now redundant with the background-color set on line 26

Copy link
Contributor

Choose a reason for hiding this comment

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

fix this please!
we are setting background color on line 26 too!
@bosiraphael wait on merge please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll pick this as cleanup in another PR @ehconitin

Copy link
Contributor

@bosiraphael bosiraphael left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for your contribution @sid0-0 :)

@bosiraphael bosiraphael merged commit 89b50c0 into twentyhq:main Sep 25, 2024
6 of 11 checks passed
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 a comment
ah nvm i was late :(

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.

Table view - New column design improvements
3 participants