-
Notifications
You must be signed in to change notification settings - Fork 9.2k
fix: separate Filter Segment implementation from DataTableProvider #20587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a932f2c
febacd7
63effee
8e569ac
7109258
18eb0f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ import { usePathname } from "next/navigation"; | |
import { useQueryState, parseAsArrayOf, parseAsJson, parseAsInteger, parseAsString } from "nuqs"; | ||
import { createContext, useCallback, useEffect, useRef, useMemo } from "react"; | ||
|
||
import { useSegments } from "./lib/segments"; | ||
import { useSegmentsNoop } from "./hooks/useSegmentsNoop"; | ||
import { | ||
type FilterValue, | ||
ZSorting, | ||
|
@@ -16,6 +16,7 @@ import { | |
ZColumnSizing, | ||
type FilterSegmentOutput, | ||
type ActiveFilters, | ||
type UseSegments, | ||
} from "./lib/types"; | ||
import { CTA_CONTAINER_CLASS_NAME } from "./lib/utils"; | ||
|
||
|
@@ -51,6 +52,7 @@ export type DataTableContextType = { | |
segmentId: number | undefined; | ||
setSegmentId: (id: number | null) => void; | ||
canSaveSegment: boolean; | ||
isSegmentEnabled: boolean; | ||
|
||
searchTerm: string; | ||
setSearchTerm: (searchTerm: string | null) => void; | ||
|
@@ -65,6 +67,7 @@ const DEFAULT_COLUMN_SIZING: ColumnSizingState = {}; | |
const DEFAULT_PAGE_SIZE = 10; | ||
|
||
interface DataTableProviderProps { | ||
useSegments?: UseSegments; | ||
tableIdentifier?: string; | ||
children: React.ReactNode; | ||
ctaContainerClassName?: string; | ||
|
@@ -74,6 +77,7 @@ interface DataTableProviderProps { | |
export function DataTableProvider({ | ||
tableIdentifier: _tableIdentifier, | ||
children, | ||
useSegments = useSegmentsNoop, | ||
defaultPageSize = DEFAULT_PAGE_SIZE, | ||
ctaContainerClassName = CTA_CONTAINER_CLASS_NAME, | ||
}: DataTableProviderProps) { | ||
|
@@ -165,36 +169,38 @@ export function DataTableProvider({ | |
[setPageSize, setPageIndex, defaultPageSize] | ||
); | ||
|
||
const { segments, selectedSegment, canSaveSegment, setSegmentIdAndSaveToLocalStorage } = useSegments({ | ||
tableIdentifier, | ||
activeFilters, | ||
sorting, | ||
columnVisibility, | ||
columnSizing, | ||
pageSize, | ||
searchTerm, | ||
defaultPageSize, | ||
segmentId, | ||
setSegmentId, | ||
setActiveFilters, | ||
setSorting, | ||
setColumnVisibility, | ||
setColumnSizing, | ||
setPageSize, | ||
setPageIndex, | ||
setSearchTerm, | ||
}); | ||
const { segments, selectedSegment, canSaveSegment, setAndPersistSegmentId, isSegmentEnabled } = useSegments( | ||
{ | ||
tableIdentifier, | ||
activeFilters, | ||
sorting, | ||
columnVisibility, | ||
columnSizing, | ||
pageSize, | ||
searchTerm, | ||
defaultPageSize, | ||
segmentId, | ||
setSegmentId, | ||
setActiveFilters, | ||
setSorting, | ||
setColumnVisibility, | ||
setColumnSizing, | ||
setPageSize, | ||
setPageIndex, | ||
setSearchTerm, | ||
} | ||
); | ||
|
||
const clearAll = useCallback( | ||
(exclude?: string[]) => { | ||
setSegmentIdAndSaveToLocalStorage(null); | ||
setAndPersistSegmentId(null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's renamed (just cleaning up) |
||
setPageIndex(null); | ||
setActiveFilters((prev) => { | ||
const remainingFilters = prev.filter((filter) => exclude?.includes(filter.f)); | ||
return remainingFilters.length === 0 ? null : remainingFilters; | ||
}); | ||
}, | ||
[setActiveFilters, setPageIndex, setSegmentIdAndSaveToLocalStorage] | ||
[setActiveFilters, setPageIndex, setAndPersistSegmentId] | ||
); | ||
|
||
const ctaContainerRef = useRef<HTMLDivElement | null>(null); | ||
|
@@ -230,8 +236,9 @@ export function DataTableProvider({ | |
segments, | ||
selectedSegment, | ||
segmentId: segmentId || undefined, | ||
setSegmentId: setSegmentIdAndSaveToLocalStorage, | ||
setSegmentId: setAndPersistSegmentId, | ||
canSaveSegment, | ||
isSegmentEnabled, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is now provided from useSegmentsNoop returns |
||
searchTerm, | ||
setSearchTerm: setDebouncedSearchTerm, | ||
}}> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ import { | |
} from "@calcom/ui/components/table"; | ||
|
||
import { useColumnSizingVars } from "../hooks"; | ||
import { usePersistentColumnResizing } from "../lib/resizing"; | ||
import { useColumnResizing } from "../hooks/useColumnResizing"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Also |
||
|
||
export type DataTablePropsFromWrapper<TData> = { | ||
table: ReactTableType<TData>; | ||
|
@@ -96,7 +96,7 @@ export function DataTable<TData>({ | |
|
||
const columnSizingVars = useColumnSizingVars({ table }); | ||
|
||
usePersistentColumnResizing({ | ||
useColumnResizing({ | ||
enabled: Boolean(enableColumnResizing), | ||
table, | ||
tableContainerRef, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ type SubmenuItem = { | |
|
||
export function FilterSegmentSelect() { | ||
const { t } = useLocale(); | ||
const { segments, selectedSegment, segmentId, setSegmentId } = useDataTable(); | ||
const { segments, selectedSegment, segmentId, setSegmentId, isSegmentEnabled } = useDataTable(); | ||
const [segmentToRename, setSegmentToRename] = useState<FilterSegmentOutput | undefined>(); | ||
const [segmentToDuplicate, setSegmentToDuplicate] = useState<FilterSegmentOutput | undefined>(); | ||
const [segmentToDelete, setSegmentToDelete] = useState<FilterSegmentOutput | undefined>(); | ||
|
@@ -99,6 +99,14 @@ export function FilterSegmentSelect() { | |
]; | ||
}, [segments, t]); | ||
|
||
if (!isSegmentEnabled) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't use these components when Segment is not enabled, but just in case of mistake, we're early returning a disabled button. |
||
return ( | ||
<Button color="secondary" StartIcon="list-filter" EndIcon="chevron-down" disabled> | ||
{t("segment")} | ||
</Button> | ||
); | ||
} | ||
|
||
return ( | ||
<> | ||
<Dropdown> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// eslint-disable-next-line no-restricted-imports | ||
import { noop } from "lodash"; | ||
|
||
import { type UseSegments } from "../lib/types"; | ||
|
||
export const useSegmentsNoop: UseSegments = ({}) => { | ||
return { | ||
segments: [], | ||
selectedSegment: undefined, | ||
canSaveSegment: false, | ||
setAndPersistSegmentId: noop, | ||
isSegmentEnabled: false, | ||
}; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It accepts
useSegments
as a prop, and we use the noop implementation as the default value.