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

feat: apply full width automatically for DataTable #18357

Merged
merged 8 commits into from
Jan 2, 2025

Conversation

eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Dec 23, 2024

What does this PR do?

This PR improves DataTable to fill the full width.

Details

How we calculate the default width of columns in DataTable:

  1. Use explicitly given size values as a part of ColumnDef. Example:
  2. Rely on getSize() function and its default value (https://tanstack.com/table/latest/docs/api/features/column-sizing#getsize)

When browser's width is too large or there is not enough columns, the DataTable doesn't fill the full width. It used to do so, but not anymore since we've applied flex to <TableRow /> for the following reasons:

  1. To enable pinned columns (on the right side)
  2. To enable column resizing

This PR fixes the logic to calculate the column widths by increasing them a bit more if the calculated total widths doesn't fill the container's clientWidth. They are also re-calculated as browser resizes.

Screenshot.2024-12-24.at.16.55.12.mp4

New behaviors:
1. initially full width
2. still full width on browser resize (but will have horizontal scroll if browser width is too small)
3. it adjusts other columns' widths automatically when resizing a column
4. it saves only intentionally resized columns in localStorage

(The recalculation of column sizes on window resize is debounced for better performance.)

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A - I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

If you already have resized columns, it may affect the result. So it's better to delete data from localStorage before testing. Delete all the keys starting with data-table-column-sizing-.

DataTable should look good in the following pages:

  • /settings/organizations/[orgSlug]/members
  • /insights/routing
  • /settings/teams/[teamId]/members

Checklist

@graphite-app graphite-app bot requested a review from a team December 23, 2024 16:11
@dosubot dosubot bot added ui area: UI, frontend, button, form, input ✨ feature New feature or request labels Dec 23, 2024
@keithwillcode keithwillcode added consumer core area: core, team members only labels Dec 23, 2024
Copy link

graphite-app bot commented Dec 23, 2024

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (12/23/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add ready-for-e2e label" took an action on this PR • (01/02/25)

1 label was added to this PR based on Keith Williams's automation.

@eunjae-lee eunjae-lee marked this pull request as draft December 23, 2024 16:24
Copy link

vercel bot commented Dec 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Jan 2, 2025 2:11pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Jan 2, 2025 2:11pm

@eunjae-lee eunjae-lee force-pushed the feat/full-width-data-table branch 6 times, most recently from 7c6cee6 to b3812fc Compare December 24, 2024 16:23
}
return colSizes;
}, [table.getFlatHeaders(), table.getState().columnSizingInfo, table.getState().columnSizing]);
const columnSizingVars = useColumnSizingVars({ table });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

extracted as a separate file for readability

@@ -543,6 +542,11 @@ function UserListTableContent() {
}
};

if (!isSuccessAttributes) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is to avoid flickering (attributes are fetched at the similar timing as the table data, anyway)

@eunjae-lee eunjae-lee force-pushed the feat/full-width-data-table branch from 5cd74fa to 8d0090e Compare December 26, 2024 10:28
@eunjae-lee eunjae-lee marked this pull request as ready for review December 26, 2024 10:29
@dosubot dosubot bot added the 🧹 Improvements Improvements to existing features. Mostly UX/UI label Dec 26, 2024
Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

Screen.Recording.2025-01-01.at.6.56.26.PM.mov

In mobile view shouldn't all columns be horizontally scrollable?

Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

Merge conflicts

@eunjae-lee
Copy link
Contributor Author

eunjae-lee commented Jan 2, 2025

Screen.Recording.2025-01-01.at.6.56.26.PM.mov

In mobile view shouldn't all columns be horizontally scrollable?

Hey @Udit-takkar , that's a good point. Currently the member column is pinned. I'll think about how/when to disable pinning feature for small viewports, and apply the change here soon.

update: fixed 10ea635

Copy link
Contributor

github-actions bot commented Jan 2, 2025

E2E results are ready!

@Udit-takkar Udit-takkar merged commit c415b2d into main Jan 2, 2025
37 checks passed
@Udit-takkar Udit-takkar deleted the feat/full-width-data-table branch January 2, 2025 14:32
MuhammadAimanSulaiman pushed a commit to hit-pay/cal.com that referenced this pull request Jan 10, 2025
* feat: apply full width automatically for DataTable

* change implementation

* load all columns of insights routing table at the same time

* update team member list

* sticky columns for >= sm

* fix type error

---------

Co-authored-by: Udit Takkar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consumer core area: core, team members only ✨ feature New feature or request 🧹 Improvements Improvements to existing features. Mostly UX/UI ready-for-e2e ui area: UI, frontend, button, form, input
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants