Skip to content

docs: clarify how to replace the sort event#684

Open
spike-rabbit wants to merge 1 commit intomainfrom
docs/clarify-sort-replacement
Open

docs: clarify how to replace the sort event#684
spike-rabbit wants to merge 1 commit intomainfrom
docs/clarify-sort-replacement

Conversation

@spike-rabbit
Copy link
Copy Markdown
Member

@spike-rabbit spike-rabbit commented Apr 29, 2026

Closes #682

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe: docs

What is the current behavior? (You can also link to an open issue here)

No hint that the page event can be a replacement for sort.

What is the new behavior?

The deprecation note states, that page should be used for server-side paging.

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@fh1ch should we also update the changelog?

@spike-rabbit spike-rabbit requested a review from a team as a code owner April 29, 2026 14:09
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the JSDoc documentation for the sort output event in DatatableComponent to provide better guidance on handling server-side paging. The review suggests improving the clarity and grammatical consistency of this documentation by using present tense and explicitly referencing the PageEvent interface properties, providing a clearer rephrasing for developers.

Comment thread projects/ngx-datatable/src/lib/components/datatable.component.ts Outdated
@spike-rabbit spike-rabbit force-pushed the docs/clarify-sort-replacement branch from 8c1332c to 35c0741 Compare April 29, 2026 15:09
@fh1ch fh1ch requested a review from Copilot April 30, 2026 12:45
@fh1ch fh1ch added the enhancement New feature or request label Apr 30, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the DatatableComponent.sort deprecation JSDoc to better explain how consumers can handle server-side paging/sorting without relying on the deprecated (sort) output.

Changes:

  • Adds a note suggesting using (page) as the single subscription point and reading PageEvent.sorts from the payload.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread projects/ngx-datatable/src/lib/components/datatable.component.ts Outdated
@spike-rabbit spike-rabbit force-pushed the docs/clarify-sort-replacement branch from 35c0741 to cf1c285 Compare May 4, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: (sortsChange) fires before (page) inside onColumnSort, breaking consumers that route on both

3 participants