Column Menu & Sort Features #147

rembrandtreyes · 2025-05-04T07:04:27Z

This PR adds an interactive column menu and improved sorting functionality to enhance the table user experience.

Key Changes:

1. New Column Menu Component

Added contextual menu for column operations
Implemented options for hiding/showing columns
Added intuitive sort controls with visual indicators for current sort state

2. Enhanced Column Headers

Added menu button that appears on hover
Positioned sort indicators next to column text
Added right-click context menu support

3. Improved Sort UX

Sort options now explicitly show "Sort ascending" and "Sort descending"
Added "Clear sort" option when a column is sorted
Visual checkmarks indicate the current sort direction
Sort indicator appears right after the column text

4. Code Quality

Added comprehensive tests for the new components
Integrated Prettier for consistent code formatting
Fixed ESLint configuration to work with Prettier

5. Technical Architecture

Created a flexible adapter pattern between menu actions and existing sorting logic
Maintained backward compatibility with click-to-sort behavior
Clean component separation with descriptive props and interfaces

This PR improves data exploration by making column operations more discoverable and providing clearer visual feedback for current table state.

Demo

Screen.Recording.2025-05-03.at.11.53.00.PM.mov

No hiding for column.length === 1

platypii · 2025-05-04T22:01:06Z

Super cool thanks @rembrandtreyes! I'll review tonight.

platypii · 2025-05-04T22:21:01Z

eslint.config.js

+import prettierPlugin from 'eslint-plugin-prettier'
+import prettierConfig from 'eslint-config-prettier'


I like prettier, I'm open to considering it. I'm not at all happy with the current eslint situation. But the change belongs in its own PR.

Yeah sounds good, I'll revert, and create an issue to add prettier in.

platypii · 2025-05-04T22:27:39Z

src/components/ColumnHeader/ColumnHeader.module.css

@@ -0,0 +1,14 @@
+.headerText {


@severo can say more but... we've been trying to use existing elements and semantic tags (aria-, data-) to style as much as possible in hightable, avoiding adding classNames as much as possible.

The problem is that people want to custom style the table. But css modules obfuscate the classNames. So to expose it outside the component you either need to allow user to pass in custom classes that are populated through the table.

The (better, so far) alternative has been to let the user specify a custom top-level className (eg- className={styles.myhightable}). Then if all the elements have semantic structure you can style from outside like:

.myhightable thead th > span { text-overflow: ellipsis; ... }

So basically I'm saying it would be better to do that here, so that users can see how we styled it, and not have to figure out weird css-affinity issues.

platypii · 2025-05-04T22:31:55Z

src/components/ColumnHeader/ColumnHeader.tsx

+        <ColumnMenuButton onClick={handleMenuButtonClick} />
+        <ColumnResizer setWidth={setWidth} onDoubleClick={autoResize} width={width} />
+      </th>
+      <ColumnMenu


Is there a reason the ColumnMenu is a sibling of the <th>? Why not inside the <th>?

I'm not necessarily opposed but doesn't this result in table that looks like:

<table> <thead> <th>...</th> <ColumnMenu/> <th>...</th> <ColumnMenu/> ...

It's the createPortal implementation that we have in ColumnMenu. So the Column menu is attached to document.body instead, this is to prevent z-index issues.

I'll move this out into a function to render here instead and I'll add a comment.

src/components/ColumnMenu/ColumnMenu.tsx

platypii · 2025-05-04T22:34:22Z

src/components/ColumnMenu/ColumnMenu.tsx

+
+  if (!isVisible) return null
+
+  return createPortal(


src/components/ColumnMenuButton/ColumnMenuButton.tsx

platypii · 2025-05-04T22:39:56Z

src/components/HighTable/HighTable.module.css

+    position: relative;
+    display: inline-block;
+    margin-left: 12px;
+    font-size: 14px;


normally would say don't use px for font-size. This is effectively an icon so I think it's okay. But if someone scale up text, should the sort indicator scale with the text?

PS- It looks really nice!

platypii · 2025-05-04T22:47:26Z

src/components/HighTable/HighTable.tsx

+                      .map((column, index) => ({ column, index }))
+                      .filter(({ index }) => isColumnVisible(index))
+                      .map(({ column, index: columnIndex }) => {


At first I was going to comment on this map/filter/map seems redundant. But as I was writing it I understood that you need the index to be stable even after the filter. So you're right. But maybe add a comment?

Yeah I wasn't sure of a "better" way to handle column visibility while preserving the original indexes. Good news is that it's still O(n) and after looking at the first map().filter() piece we can useMemo to prevent unnecessary recalculations. The bad news is trying to grok this, and memory consideration with extremely large tables (column dependent).

I'll add these lines:

// We use map/filter/map to preserve original column indexes after filtering: // 1. First map: Annotate each column with its original index // 2. Filter: Remove hidden columns while keeping original index information // 3. During render: Use the preserved originalIndex to access properties that // depend on the column's position in the original array (CSS classes, event handlers, etc.)

platypii · 2025-05-04T22:48:36Z

src/components/TableHeader/TableHeader.tsx

+        if (sortDirection !== undefined) {
+          // Handle explicit sort directions from the menu
+          const { prefix, suffix } = partitionOrderBy(orderBy, columnHeader);
+
+          if (sortDirection === null) {


What's up with undefined in one branch and null in another?

sortDirection !== undefined separates header clicks from column menu actions

(sortDirection === null) handles the clear sort case within the column menu

I'll add better comments as it isn't very clear that the sortDirection is undefined for header clicks.

platypii · 2025-05-04T22:49:30Z

src/components/TableHeader/TableHeader.tsx

+              onOrderByChange([{ column: columnHeader, direction: 'descending' }, ...suffix]);
+            } else {
+              // descending -> none
+              onOrderByChange([...suffix]);


no semicolons please... npm run lint:fix

Co-authored-by: Kenny Daniel <[email protected]>

rembrandtreyes · 2025-05-14T15:11:30Z

Hey folks. Just got back from vacation. I'll be working through these comments today and rerequest a review. Thanks again @severo for great feedback!

…isabled prop for better control over column visibility. Refactor styles and logic for improved clarity and functionality in the column menu.

…pdate HighTableContext to provide a default portal target. Simplify TableHeader by removing unnecessary hasHiddenColumns logic.

…y sorting functionality by replacing legacy props with a single onSort prop. Simplify event handling for sorting actions and enhance clarity in column visibility management.

…tions. Remove legacy sorting props and ensure correct event handling for sort and clear sort functionalities.

…mnMenu

…ing padding values and adding a ref prop for improved table reference handling.

…abIndex of the ColumnMenuButton to -1 for improved accessibility. Reorder ColumnMenuButton rendering in ColumnHeader for better structure.

…ity by renaming TableProps to TableHeaderProps and removing unnecessary comments. Adjust import statements for consistency.

…ed layout flexibility.

rembrandtreyes · 2025-05-16T06:07:10Z

Ok, I was able to merge in latest master and work through the conflicts 😅. I think I have everything included. I tested what the UX is in master and compared it to my branch and everything seems at parity for keyboard nav. At least the unit tests are happy 😄

I did not include the ColumnMenu in the keyboard nav because I think that can be its own issue/PR.

Just combing through my PR now and making sure everything is aligned.

…nnecessary re-renders, enhancing performance through prop comparison.

rembrandtreyes · 2025-05-16T07:54:27Z

Damn I keep forgetting to check npm run typecheck fixing in a bit.

… and ensure proper cleanup when the menu is closed.

severo · 2025-05-16T08:06:08Z

It's ready for review, right? Looking at it

…dren prop to prevent unnecessary re-renders.

rembrandtreyes · 2025-05-16T08:09:24Z

Yeah its ready now

severo · 2025-05-16T08:13:31Z

After closing the menu clicking outside, the table is stuck:

Screencast.From.2025-05-16.10-11-15.mp4

~~Somewhat related: we should be able to close the menu by clicking the "..." button.~~ <= already reported above, I can take care of this comment later. But the other comment (table is stuck) is a bug we have to fix here.

severo · 2025-05-16T08:17:13Z

~~We should always give a bit more space when a column is sortable (like in master). Otherwise, the sort arrow is hidden by default (note that it appears when clicking autoresize)~~ <= edit: I already mentioned above that I can fix it later. So feel free to ignore in this PR.

Screencast.From.2025-05-16.10-15-23.mp4

rembrandtreyes · 2025-05-16T08:48:57Z

@severo just saw your update 😆 I just called autoresize() when we cycle through sort states. What do you think?

autoresize.mov

severo · 2025-05-16T08:51:06Z

Hmmm, no, I think sorting and resizing should be independent. We persist the column sizes in local storage, and we don't want to autoresize when sorting, it's not related. We should leave enough space in CSS instead.

rembrandtreyes · 2025-05-16T09:01:42Z

@severo I just checked master and I am seeing the same issue reported about clicking outside the columns in the whitespace. It appears to be selecting the table scroll container? Only way I can "escape" the lock is tabbing to the next item.

severo · 2025-05-16T09:04:39Z

Indeed, I reproduce incoherencies in master. Thanks for checking, I'll take care of it in another PR.

edit: I created #167

severo

I think we have several big features in this PR, and it's slowing us down.

Are you OK with opening new PRs instead, so we can focus on each one?

Feature: column menu #129 - create a menu element with no new action (the only action would be: toggle column sort, which is a bit silly, but would allow to see if it works as expected). We would ensure the keyboard works as expected and the portal is working well.
add actions related to sorting (sort ascending, descending, remove from sort) to the menu. Taking care of handling the sort in sort.ts
Feature: hide columns #3 - add the feature to hide columns.
Use new icons for sort? #136 - change the sort icons

Sorry for asking for that now, I should have asked before, to save time for everyone. But I think it will help a lot with the integration. Again: thanks for your time, the features are super useful!

severo · 2025-05-16T08:52:58Z

src/components/HighTable/HighTable.tsx

@@ -569,6 +616,8 @@ export function HighTableInner({
            </tbody>
          </table>
        </div>
+        {/* puts a background behind the row labels column */}
+        <div className={styles.mockRowLabel} style={cornerStyle}>&nbsp;</div>


it seems to be duplicated code (see two lines below)

severo · 2025-05-16T09:08:43Z

src/components/TableHeader/TableHeader.tsx

-  const getOnOrderByClick = useCallback((columnHeader: string) => {
-    if (!onOrderByChange || !orderBy) return undefined
-    return () => {
-      onOrderByChange(toggleColumn(columnHeader, orderBy))


This comment should be fixed.

rembrandtreyes · 2025-05-18T23:08:17Z

Alright I just pumped out #174 I'll close this PR in favor of the three we discussed @severo. Thanks again for taking the time to review!

rembrandtreyes force-pushed the rembrandtreyes/ColumnMenu branch from 9c7e160 to 92578c0 Compare May 4, 2025 07:26

platypii requested review from severo and platypii May 4, 2025 22:00

platypii reviewed May 4, 2025

View reviewed changes

This was referenced May 5, 2025

Use new icons for sort? #136

Closed

Feature: column menu #129

Open

rembrandtreyes and others added 22 commits May 6, 2025 01:02

Format ColumnHeader

0b4a40c

Add ColumnMenu to ColumnHeader

3704a3b

Format TableHeader

e8b4457

Add ColumnMenuButton functionality

8b3aab6

Scaffold hidecolumn functionality

34f6fcc

Implement hiding/showing functionality

03fd7ba

Skip row error check when columns are hidden

9db06ec

Fix column header styles when column menu button is shown/hidden

5e67cb9

Fixup styles and colors to match current theme

19baffc

Add tests for new components

a414121

Add sort functionality to the menu

ecc6f82

Add menu to columnheader test

9ee1ce6

Remove sort -> Clear sort

b11d4ca

Prettier

2c1e4c8

fix type error and lint error

447cf3f

Style changes

018db93

newlines

f26ee02

fix rebase issue

a8ad0a0

Update src/components/ColumnMenu/ColumnMenu.tsx

48282d5

Co-authored-by: Kenny Daniel <[email protected]>

Update src/components/ColumnMenuButton/ColumnMenuButton.tsx

1803795

Co-authored-by: Kenny Daniel <[email protected]>

PR comments

b0e0f42

npm run lint:fix

271c8c3

rembrandtreyes force-pushed the rembrandtreyes/ColumnMenu branch from 661a9ef to 271c8c3 Compare May 6, 2025 08:43

rembrandtreyes added 12 commits May 15, 2025 10:02

Enhance ColumnMenu and ColumnHeader components by introducing isHideD…

e2edc76

…isabled prop for better control over column visibility. Refactor styles and logic for improved clarity and functionality in the column menu.

Refactor ColumnMenu test to correctly utilize onHideColumn prop and u…

c2f785c

…pdate HighTableContext to provide a default portal target. Simplify TableHeader by removing unnecessary hasHiddenColumns logic.

Refactor ColumnHeader, ColumnMenu, and TableHeader components to unif…

3942dd1

…y sorting functionality by replacing legacy props with a single onSort prop. Simplify event handling for sorting actions and enhance clarity in column visibility management.

Update ColumnMenu tests to utilize unified onSort prop for sorting ac…

8ab6289

…tions. Remove legacy sorting props and ensure correct event handling for sort and clear sort functionalities.

Merge remote-tracking branch 'origin/master' into rembrandtreyes/Colu…

000b7ff

…mnMenu

Update HighTable styles and enhance component functionality by adjust…

c376432

…ing padding values and adding a ref prop for improved table reference handling.

Refactor ColumnHeader and ColumnMenuButton components to adjust the t…

04f39a8

…abIndex of the ColumnMenuButton to -1 for improved accessibility. Reorder ColumnMenuButton rendering in ColumnHeader for better structure.

Refactor TableHeader and ColumnHeader components to improve code clar…

60fef22

…ity by renaming TableProps to TableHeaderProps and removing unnecessary comments. Adjust import statements for consistency.

Remove fixed positioning and zIndex from ColumnMenu styles for improv…

48c8284

…ed layout flexibility.

Remove relative style from column header

cb463e6

Remove duplicate props and revise column naming in column menu

de177f3

Fix test

3378fff

Optimize ColumnHeader component by wrapping it with memo to prevent u…

4656f03

…nnecessary re-renders, enhancing performance through prop comparison.

rembrandtreyes added 2 commits May 16, 2025 01:00

Add missing types

2da7876

Refactor event listener management in ColumnHeader to improve clarity…

43f2fef

… and ensure proper cleanup when the menu is closed.

Enhance prop comparison in ColumnHeader memoization by including chil…

0a38e2f

…dren prop to prevent unnecessary re-renders.

severo requested changes May 16, 2025

View reviewed changes

rembrandtreyes closed this May 18, 2025

		import prettierPlugin from 'eslint-plugin-prettier'
		import prettierConfig from 'eslint-config-prettier'

Column Menu & Sort Features #147

Column Menu & Sort Features #147

Uh oh!

Conversation

rembrandtreyes commented May 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Changes:

1. New Column Menu Component

2. Enhanced Column Headers

3. Improved Sort UX

4. Code Quality

5. Technical Architecture

Demo

No hiding for column.length === 1

Uh oh!

platypii commented May 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

platypii May 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rembrandtreyes commented May 14, 2025

Uh oh!

rembrandtreyes commented May 16, 2025

Uh oh!

rembrandtreyes commented May 16, 2025

Uh oh!

severo commented May 16, 2025

Uh oh!

rembrandtreyes commented May 16, 2025

Uh oh!

severo commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

severo commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rembrandtreyes commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

severo commented May 16, 2025

Uh oh!

rembrandtreyes commented May 16, 2025

Uh oh!

severo commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

severo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rembrandtreyes commented May 18, 2025

rembrandtreyes commented May 4, 2025 •

edited

Loading

platypii May 4, 2025 •

edited

Loading

severo commented May 16, 2025 •

edited

Loading

severo commented May 16, 2025 •

edited

Loading

rembrandtreyes commented May 16, 2025 •

edited

Loading

severo commented May 16, 2025 •

edited

Loading

severo left a comment •

edited

Loading