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.

Sign up for GitHub

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

Jump to bottom

feat: Column title menus #1616

Merged
merged 15 commits into from
Sep 19, 2023
Merged

feat: Column title menus #1616

merged 15 commits into from
Sep 19, 2023

Conversation

AdityaPimpalkar
Copy link
Contributor

@AdityaPimpalkar AdityaPimpalkar commented Sep 16, 2023 •
edited

closes: #1524

Screen.Recording.2023-09-16.at.5.03.55.PM.mov

@AdityaPimpalkar
Copy link
Contributor Author

AdityaPimpalkar commented Sep 16, 2023 •
edited

There is a weird glitch happening after I press the update view button, it goes back to the original position, after hard refresh I can see it's updating in db. Maybe I am missing something in my logic?

@ergomake
Copy link

ergomake bot commented Sep 16, 2023 •
edited

Hi 👋

We couldn't create a preview environment for this pull-request 😥

You can see your environment build logs here. Please double-check your docker-compose.yml file is valid.

If you need help, email us at contact@getergomake.com or join Discord.

Click here to disable Ergomake.

AdityaPimpalkar
AdityaPimpalkar commented Sep 16, 2023
View reviewed changes
Copy link
Contributor Author

@AdityaPimpalkar AdityaPimpalkar left a comment

Choose a reason for hiding this comment

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

Explained the logic a bit, hope it helps! :)

charlesBochet
charlesBochet reviewed Sep 16, 2023
View reviewed changes
Copy link
Member

@charlesBochet charlesBochet left a comment •
edited

Choose a reason for hiding this comment

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

Great! Very nice start!
I have left comments

Thanks a lot :)

server/src/database/migrations/20230727124244_add_view_fields_table/migration.sql Outdated Show resolved Hide resolved
front/src/modules/ui/table/components/ColumnHead.tsx Outdated Show resolved Hide resolved
front/src/modules/ui/table/components/ColumnHead.tsx Outdated Show resolved Hide resolved
front/src/modules/ui/table/components/ColumnHead.tsx Outdated Show resolved Hide resolved
front/src/modules/ui/table/components/EntityTableHeaderOptions.tsx Show resolved Hide resolved
front/src/modules/ui/table/components/EntityTableHeaderOptions.tsx Outdated Show resolved Hide resolved
front/src/modules/ui/table/hooks/useTableColumns.ts Outdated Show resolved Hide resolved
front/src/modules/ui/table/hooks/useTableColumns.ts Outdated Show resolved Hide resolved
front/src/modules/ui/table/hooks/useTableColumns.ts Outdated Show resolved Hide resolved
front/src/modules/ui/table/states/selectedTableColumnHeaderState.ts Outdated Show resolved Hide resolved
@charlesBochet
Copy link
Member

There is a weird glitch happening after I press the update view button where it goes back to the original state before moving the column, after hard refresh I can see it's updating in db. Maybe I am missing something in my logic

Yes, this is tied to optimistic rendering issues. You can ignore them in this PR, I'll take a look once merged, I want to test a global strategy on this one :)

@FelixMalfait
Copy link
Member

Really impressed @AdityaPimpalkar! Thanks a lot for taking on this big task!

AdityaPimpalkar
AdityaPimpalkar commented Sep 17, 2023
View reviewed changes
Copy link
Contributor Author

@AdityaPimpalkar AdityaPimpalkar left a comment •
edited

Choose a reason for hiding this comment

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

Need a little help with this, thanks! :)

@AdityaPimpalkar AdityaPimpalkar mentioned this pull request Sep 19, 2023
Closed
charlesBochet
charlesBochet approved these changes Sep 19, 2023
View reviewed changes
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 @AdityaPimpalkar, it looks good, there are a few things I want to change. i'm merging it and I will do a quick follow PR and post it there!

Good piece!

server/src/database/migrations/20230727124244_add_view_fields_table/migration.sql Outdated Show resolved Hide resolved
server/src/database/migrations/20230727124244_add_view_fields_table/migration.sql Outdated Show resolved Hide resolved
front/src/modules/views/hooks/useTableViews.ts Show resolved Hide resolved
@charlesBochet charlesBochet merged commit 321488a into twentyhq:main Sep 19, 2023
6 of 8 checks passed
@charlesBochet charlesBochet mentioned this pull request Sep 19, 2023
Merged
@charlesBochet
Copy link
Member

@AdityaPimpalkar https://github.com/twentyhq/twenty/pull/1665/files here they are! thanks again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@charlesBochet charlesBochet charlesBochet approved these changes

Assignees
No one assigned
Labels
PR: awaiting author
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

People & Companies column title menus
3 participants
@AdityaPimpalkar @charlesBochet @FelixMalfait