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

Fixes #7105: Fixed column header on Kanban boards #7263

Merged
merged 9 commits into from
Oct 16, 2024

Conversation

dilshad-knk
Copy link
Contributor

Issue Reference: Fixes #7105

Description:
This pull request introduces adjustments to the styling of the RecordBoardColumnHeader component. The modifications enhance the layout and visual consistency of the Kanban board headers.

Changes Made:
Margin Adjustment:

Increased the bottom margin from theme.spacing(2) to theme.spacing(6) for better spacing below the header.
Header Container Enhancements:

Added a background color sourced from the theme (theme.background.primary) to the header container for improved visibility and aesthetics.
Set a fixed height of 40px for the header to ensure a consistent size across different screens.
Applied a fixed position to the header container to keep it visible at the top during scrolling.
Added padding at the top using theme.spacing(2) for better alignment of content within the header.

Before :

368248677-c6649976-62f2-4b20-b4d3-9684c091776c.mp4

Now :

now.webm

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 issue #7105 by fixing the column header on Kanban boards, improving visual consistency and usability.

  • Added position: fixed to StyledHeaderContainer in RecordBoardColumnHeader.tsx to keep headers visible while scrolling
  • Set a consistent height of 40px for the header container
  • Added background color from theme to header container for improved visibility
  • Increased bottom margin from theme.spacing(2) to theme.spacing(6) for better spacing below the header
  • Added top padding using theme.spacing(2) for better content alignment within the header

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

@ehconitin
Copy link
Contributor

ehconitin commented Sep 26, 2024

@dilshad-knk Thank you for the PR! However, I believe this may not be the right approach.

The hover button in the header isn't functioning as expected, and scrolling still affects the tabs above the headers (specifically the view/filter tab).

Take your time with this. A more elegant solution might be to find a way to exclude those tabs/components from the RecordBoards scrollWrapper, keeping only the columns within it.

That said, I'm not sure if the hint I gave would work, so feel free to reach out if you have any questions or need further clarification!

Also have a look at how its implemented for tables!

2024-09-26.14-24-27.mp4

justify-content: space-between;
position: fixed;
Copy link
Contributor

@ehconitin ehconitin Sep 26, 2024

Choose a reason for hiding this comment

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

I think this could be why the hover buttons are not working as expected!

@ehconitin
Copy link
Contributor

@dilshad-knk are you still working on it?

@Bonapara
Copy link
Member

Bonapara commented Oct 9, 2024

Hi @dilshad-knk, thanks for raising this PR. A few feedbacks:

CleanShot.2024-10-09.at.11.35.51.mp4

The column header bar should be sticky, not fixed (It should not goes under the view bar when trying to scroll top for instance)

CleanShot.2024-10-09.at.11.38.45.mp4

Box shadows should not be visible when the card passes behind the column header. (The column header should take the full column width)

CleanShot 2024-10-09 at 11 43 10

The view bar border bottom should be visible (Figma image above)

CleanShot 2024-10-09 at 11 43 57

The header bar extends beyond the container on the left side.

Figma

https://www.figma.com/design/xt8O9mFeLl46C5InWwoMrN/Twenty?node-id=40088-434680&node-type=frame&t=emAJEVbp4fn8hzHL-11

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.

LGTM

@ehconitin
Copy link
Contributor

@charlesBochet
I noticed that the margins between columns does not extend to the bottom :(

Screenshot 2024-10-16 at 15 28 22

@charlesBochet
Copy link
Member

Thank you @dilshad-knk and @lucasbordeau

@charlesBochet
Copy link
Member

/award 300

Copy link

oss-gg bot commented Oct 16, 2024

Awarding dilshad-knk: 300 points 🕹️ Well done! Check out your new contribution on oss.gg/dilshad-knk

@charlesBochet charlesBochet merged commit 720fe32 into twentyhq:main Oct 16, 2024
10 of 11 checks passed
Copy link

Thanks @dilshad-knk for your contribution!
This marks your 1st PR on the repo. You're top 49% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

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.

Fixed column header on Kanban boards
5 participants