⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@HenriRabalais
Copy link
Collaborator

Description

This PR performs a migration for the FilterableDataTable component to class component.

Key Changes

1. TypeScript Migration & Architecture

  • New Type System: Created jsx/types.ts to centralize all table-related types.
  • Type Intersection: Implemented FilterableDataTableProps as an intersection of DataTableProps. This ensures the wrapper "englobes" all base table properties (like actions, data, and getFormattedCell) without duplication, ensuring synchronization between parent and child components.
  • Legacy Support: Added declarations.d.ts to provide module definitions for existing JavaScript components (Filter.js, ProgressBar.js, etc.), resolving TS7016 "Implicit Any" compiler errors.

2. Logic & State Improvements

  • URL Persistence: Added a debounced useEffect (300ms) to synchronize the filter state with the browser's URL query parameters via window.history.replaceState.
  • Filter Hydration: Implemented logic to parse and "re-hydrate" filter states from the URL on initial page load, allowing for bookmarkable filtered views.
  • Validation: Refined the validFilters memoization to ensure only fields marked for filtering are passed to the UI, preventing unnecessary re-renders.

How to Test

  1. Initial Load: Navigate to the table; it should load with no filters.
  2. Filtering: Apply a filter (e.g., Search Name). Verify:
    • The table filters immediately.
    • The URL updates with ?filters=... after the 300ms debounce.
  3. Persistence: Refresh the browser with active filters in the URL. Verify the filters are automatically reapplied.
  4. Clearing: Click the "Clear All" button. Verify the table resets and the URL parameters are removed.

@github-actions github-actions bot added the Language: Javascript PR or issue that update Javascript code label Jan 27, 2026
@HenriRabalais HenriRabalais changed the title [JSX] Migrate FilterableDataTable to TypeScript and Centralize Table Types [JSX] Convert FilterableDataTable to Functional Component Jan 27, 2026
@@ -0,0 +1,4 @@
declare module 'jsx/Filter';
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this file for?

Copy link
Collaborator Author

@HenriRabalais HenriRabalais Jan 30, 2026

Choose a reason for hiding this comment

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

I was having trouble making use of other jsx files that don't have typescript declarations (i.e. ProgressBar.js, Filter.js), and this was the solution I came upon.

I'm not sure why I included Datatable and Panel, they seem to have sidecar typescript declarations.

I can find another solution or alternatively convert those to .tsx in this PR as well.

@@ -0,0 +1,66 @@
import {ReactNode} from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like the idea of having a different "types" in jsx.. is there an existing file that should own each of these?

Copy link
Collaborator Author

@HenriRabalais HenriRabalais Jan 30, 2026

Choose a reason for hiding this comment

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

Ah, okay! I was trying to follow the lead from the dataquery module where I noticed a types.js file

One of the reasons this seemed useful is because the Props for FilterableDatatable will essentially be a union of the Props for Datatable and Filter and a types.js file felt like a convenient central place to store them without having to import across files.

Do you suggest I just declare the types at the top of the respective components?

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

Labels

Language: Javascript PR or issue that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants