Skip to content

fix(datatable #5633 #6764): preserve keyless columns during reorder - #8261

Open
ealipkin wants to merge 1 commit into
primefaces:masterfrom
ealipkin:fix/datatable-reorder-null-columns
Open

ealipkin wants to merge 1 commit into
primefaces:masterfrom
ealipkin:fix/datatable-reorder-null-columns

Conversation

@ealipkin

Copy link
Copy Markdown
  • fix(datatable): preserve keyless columns during reorder
  • Resolves #6764 and #5633
  • Scope: DataTable reorderable-columns logic only
  • Root cause: columns() rebuilt the ordered list purely from d_columnOrder. findColumnByKey treated null order entries as a match for the first column with a falsy key, so selector or frozen columns were duplicated when d_columnOrder contained null placeholders. The final spread (...cols.filter) then re-appended the original keyed column, causing copies to appear after each drag. The same logic also forced keyless columns to the tail of the array when null was used to reserve their position.
  • Fix: tightened findColumnByKey to ignore null or undefined props and process d_columnOrder with a queue of keyless columns so each null placeholder consumes exactly one selector or frozen column in its original sequence. Hidden columns remain skipped and the remainder filter no longer re-adds duplicates.
  • Verification: manual testing with selector plus multi reorder, frozen action column, and multiple null placeholders at the start and end of d_columnOrder. Column count stays stable, keyless columns remain anchored, and hidden columns are still excluded.

@Coderxrohan Coderxrohan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a really solid fix — thanks for the detailed explanation as well.

I’ve dealt with this exact edge case in PrimeVue DataTable before, and your root cause analysis around d_columnOrder + null placeholders matches what I’ve seen in practice. Tightening findColumnByKey to ignore null / undefined keys and then consuming keyless columns via a queue feels like the right approach.

I also like that:

  1. Keyless (selector / action / frozen) columns stay anchored
  2. Hidden columns remain excluded
  3. Duplicate columns are no longer re-appended after reorder

The change is nicely scoped to reorderable-columns logic and doesn’t affect normal keyed columns. From a quick review, I don’t see any behavioral regressions, and the verification scenarios you listed cover the risky cases.

From my side, this looks safe to merge and a good fix for a long-standing issue

This branch has not been deployed

No deployments
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.

Datatable: reorderableColumns

2 participants