Skip to content

fix: column sort indications reflect truth [MA-4062]#2808

Open
Chalks wants to merge 1 commit intomainfrom
fix/table-sort-ma-4062
Open

fix: column sort indications reflect truth [MA-4062]#2808
Chalks wants to merge 1 commit intomainfrom
fix/table-sort-ma-4062

Conversation

@Chalks
Copy link
Contributor

@Chalks Chalks commented Jun 27, 2025

Satisfies MA-4062

I'm a little bit opinionated in this PR and I haven't run this past design. Very willing to change anything that's in here, particularly the style choices.


Issues that are addressed by this PR

  1. column headers that are sortable always initially show a downward pointing arrow no matter what the actual state of the data is.
  2. multi-column sorting is not supported out of the box but when multiple columns are sortable it looks like it is, particularly before you click on any header

Description of changes

To address those two concerns, I made two changes. First, we use a different icon to indicate that a column could be sorted. Second, you can provide an initialSort in your header to make the arrow initialized in the direction you want it to appear.

I haven't run these changes past design, so I'm happy to change anything about the styles of this.

Screenshots

arrow position on initial load

before

Screenshot 2025-06-27 at 3 21 35 PM

after

Screenshot 2025-06-27 at 3 40 50 PM

arrow position after clicking on a column

before

Screenshot 2025-06-27 at 3 44 24 PM

after

Screenshot 2025-06-27 at 3 43 41 PM

Initial load with the initialSort set to 'asc'

Screenshot 2025-06-27 at 3 45 47 PM

@Chalks Chalks requested review from a team, Justineo, adamdehaven and jillztom as code owners June 27, 2025 19:47
@netlify
Copy link

netlify bot commented Jun 27, 2025

Deploy Preview for kongponents-sandbox ready!

Name Link
🔨 Latest commit 7bb38fd
🔍 Latest deploy log https://app.netlify.com/projects/kongponents-sandbox/deploys/685ef5c0b4c12b0008b04c20
😎 Deploy Preview https://deploy-preview-2808--kongponents-sandbox.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@Chalks Chalks force-pushed the fix/table-sort-ma-4062 branch from 24952a5 to 7bb38fd Compare June 27, 2025 19:49
@kongponents-bot
Copy link
Collaborator

Preview package from this PR in consuming application

In consuming application project install preview version of kongponents generated by this PR:

@kong/kongponents@pr-2808

@netlify
Copy link

netlify bot commented Jun 27, 2025

Deploy Preview for kongponents ready!

Name Link
🔨 Latest commit 24952a5
🔍 Latest deploy log https://app.netlify.com/projects/kongponents/deploys/685ef5564d787b0008530b18
😎 Deploy Preview https://deploy-preview-2808--kongponents.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link

netlify bot commented Jun 27, 2025

Deploy Preview for kongponents ready!

Name Link
🔨 Latest commit 7bb38fd
🔍 Latest deploy log https://app.netlify.com/projects/kongponents/deploys/685ef5c0d7e95900087e9af1
😎 Deploy Preview https://deploy-preview-2808--kongponents.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

/** this property defines whether sort icon should be displayed next to the column header and whether the column header will emit sort event upon clicking on it */
sortable?: boolean
/** When provided, determines the intial arrow direction on the column header, if more than one header has this value only the first is applied */
initialSort?: 'asc' | 'desc'
Copy link
Member

Choose a reason for hiding this comment

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

There's already a sortColumnOrder prop exposed via tablePreferences - shouldn't that be sufficient?

/** this property defines whether sort icon should be displayed next to the column header and whether the column header will emit sort event upon clicking on it */
sortable?: boolean
/** When provided, determines the intial arrow direction on the column header, if more than one header has this value only the first is applied */
initialSort?: 'asc' | 'desc'
Copy link
Member

Choose a reason for hiding this comment

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

There's already a sortColumnOrder prop exposed via tablePreferences - shouldn't that be sufficient?

/** This property defines whether sort icon should be displayed next to the column header and whether the column header will emit sort event upon clicking on it */
sortable?: boolean
/** When provided, determines the intial arrow direction on the column */
initialSort?: SortColumnOrder
Copy link
Member

Choose a reason for hiding this comment

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

There's already a sortColumnOrder prop exposed via tablePreferences - shouldn't that be sufficient?

@adamdehaven
Copy link
Member

More discussion on Slack

@portikM
Copy link
Member

portikM commented Jul 14, 2025

Issues this PR aimed to fix are being fixed in #2819

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants