Skip to content
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

feat!: implement group by list (#302) #303

Merged
merged 26 commits into from
Jan 3, 2025
Merged

feat!: implement group by list (#302) #303

merged 26 commits into from
Jan 3, 2025

Conversation

frano-m
Copy link
Contributor

@frano-m frano-m commented Dec 18, 2024

Closes #302.

@frano-m frano-m force-pushed the fran/302-group-by-list branch from 0449a9d to f0b399a Compare December 23, 2024 08:30
* @param row - Row.
* @returns row identifier.
*/
function getRowId<T extends RowData>(row: Row<T>): string | undefined {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duplicated function - moved getRowId to "TableRows/utils".

@@ -44,6 +45,7 @@ export const GridTable = styled(MTable, {
background-color: ${smokeLightest};

td {
${textBodySmall500};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Styled updated as per mock for grouped column.

@@ -6,6 +6,7 @@ import { RowPositionCell } from "../components/TableCell/components/RowPositionC
export const COLUMN_DEF: Record<string, ColumnDef<RowData>> = {
ROW_POSITION: {
cell: RowPositionCell,
enableGrouping: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a default, it's unlikely this column will ever be "grouped".

* @param sortDirection - Column sort direction.
* @returns the coumn sort direction.
*/
export function getColumnSortDirection(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getColumnSortDirection is now done directly on the component.

disabled: initialVisibilityState[id],
label: header as string, // TODO revisit type assertion here
onChange: () => {
handleToggleVisibility(table, column);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the onChange function to handleToggleVisibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: The getEditColumnOptions function will be updated as part of the fix for column visibility.

columns: ColumnConfig[],
defaultSort: ColumnSort | undefined
export function getInitialState<T extends RowData>(
columns: ColumnConfig<T>[]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ColumnConfig interface is updated with generic.

@@ -243,6 +222,7 @@ export function getGridTemplateColumns<T extends RowData>(
columns: Column<T>[]
): string {
return columns
.filter(filterGroupedColumn)
Copy link
Contributor Author

@frano-m frano-m Dec 24, 2024

Choose a reason for hiding this comment

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

We filter out any grouped column from the gridTemplateColumns value - the table renders a grouped column as a row.

): InitialTableState {
const columnVisibility = getInitialTableColumnVisibility(columns);
const sorting = getInitialTableStateSorting(defaultSort);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing sorting from the function getInitialState; we want to manage initial state from table options and as we are dealing with table sorting as part of this feature, I have ensured the initial sorting state can only be defined in table options.

* @param column - Table column.
* @returns table sort label props.
*/
export function getTableSortLabelProps<T extends RowData>(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getTableSortLabelProps is deleted and the props are added directly to the TableSortLabel component in TableHead.

* @param sortDirection - Column sort direction.
* @returns true when column has a sort direction.
*/
export function isColumnSortActive(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

isColumnSortActive is added directly to TableSortLabel component.

if (!enableSorting) return;

// Column sorting is enabled.
if (getCanSort()) {
Copy link
Contributor Author

@frano-m frano-m Dec 24, 2024

Choose a reason for hiding this comment

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

We check if the column that is to be "grouped" can also be sorted getCanSort; and if so, on grouping, we always sort it as first sort. We then append any initial sorting state to the sorting state.

}

// Column sorting is not enabled; reset sorting state to initial state.
resetSorting();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The column that is to be "grouped" cannot be sorted, so reset sorting to initial sort state.

// Column is visible.
// Table grouping is enabled, and column is grouped.
// We can have the table TODO table grouping not enabled, but still have a column grouped!
if (enableGrouping && getIsGrouped()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unlikely to occur, but we check that if column we no longer wish to view, isn't the grouped column. If it is, we reset grouping state to [] and clear the grouped column from sorting state.

// Sorting is enabled.
// Table is not grouped; toggle sorting as usual.
if (grouping.length === 0) {
toggleSorting(undefined, isMultiSortEvent?.(mouseEvent));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Table isn't grouped; sort as normal.

// Table is grouped.
// Multi-sort mode and multi-sort requested; toggle sorting as usual.
if (enableMultiSort && isMultiSortEvent?.(mouseEvent)) {
toggleSorting(undefined, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Table is grouped, but user has requested multi-sort; sort as normal (the "grouped" column is either in first sort position, or if is not sortable, ignored anyway).

// Column to be sorted is not the last most recent sorted column.
if (getSortIndex() !== sorting.length - 1) {
// Reset sorting state, preserving grouped column as the first sort, and requested column to be sorted as the second sort.
setSorting([buildColumnSort(groupedColumn), buildColumnSort(column)]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the grouped column is sorted, and multi-sort is enabled, and the user has requested a sort on a column that was not the last-sorted-column, we want to refresh the sort (as if it is a newly requested sort).

The grouped column can be sorted, and so, we assumed, as it is grouped, and sorted, that the user will want to view the grouped column sorted and so we preserve the grouped column as first sort and the "requested" sort column as second sort.

}
} else {
// Space case; in single-sort mode we do not override a grouped sort.
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The grouped column is sorted, but we are in single-sort mode, and so the action is to do nothing, because we are unable to sort more than a column at a time.

}
}

toggleSorting();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The grouped column isn't sorted, so the sort is requested, as per usual - as a new sort.


export const TableHead = <T extends RowData>({
rowDirection,
tableInstance,
}: TableHeadProps<T>): JSX.Element => {
const {
options: { enableSortingInteraction },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

enableSortingInteraction is now on table options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

enableSortingInteraction allows the table to be sortable, but controls whether or not the user can interact with sorting i.e. view sorting on a column, or interact with table sorting.

if (enableMultiSort) return false;
// Single-sort mode; grouped column is sorted.
const groupedColumn = getColumn(grouping[0]);
return !!groupedColumn?.getIsSorted();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The table is in single-sort mode, is grouped, and the grouped column is/is not sorted already. Returns a sort value to determine whether or not the sort label is disabled. A table in single-sort mode with a sorted, grouped column should not be able to be sortable any further.

{...MENU_PROPS}
button={(props) => (
<DropdownButton {...props}>
{getButtonLabel(groupingByColumnId, grouping)}
Copy link
Contributor Author

@frano-m frano-m Dec 24, 2024

Choose a reason for hiding this comment

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

DropdownButton label is updated, depending on grouping state. This update, has meant we needed to update how we pass in the button for DropdownMenu. See changes and other updates here.

import { EXPANDED_OPTIONS } from "./constants";

export function useExpandedOptions<T extends RowData>(): ExpandedOptions<T> {
return { ...EXPANDED_OPTIONS };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Scaffolding structure, set up and ready for when we want to implement controlling expansion of rows.

Copy link
Contributor Author

@frano-m frano-m Dec 24, 2024

Choose a reason for hiding this comment

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

We are starting to move table option config into a centralised location, and these are some of the sorting options that are hard-coded into the table - for now.

track(EVENT_NAME.ENTITY_TABLE_SORTED, {
[EVENT_PARAM.ENTITY_NAME]: exploreState.tabValue,
[EVENT_PARAM.COLUMN_NAME]: sorting[0].id,
[EVENT_PARAM.SORT_DIRECTION]: sorting[0].desc
[EVENT_PARAM.COLUMN_NAME]: sorting[0]?.id,
Copy link
Contributor Author

@frano-m frano-m Dec 24, 2024

Choose a reason for hiding this comment

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

Sometimes sorting could be [];

string,
BaseColumnConfig<RowData, unknown>
> = {
export const COLUMN_CONFIGS: Record<string, ColumnConfig<RowData>> = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating COLUMN_CONFIGS to use standard TanStack API.

Fran McDade added 24 commits December 31, 2024 16:31
@frano-m frano-m marked this pull request as ready for review January 3, 2025 00:40
@frano-m frano-m changed the title feat: implement group by list (#302) feat!: implement group by list (#302) Jan 3, 2025
Copy link
Collaborator

@NoopDog NoopDog left a comment

Choose a reason for hiding this comment

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

Amaze!

@NoopDog NoopDog merged commit 36e4330 into main Jan 3, 2025
2 checks passed
@frano-m frano-m deleted the fran/302-group-by-list branch January 3, 2025 23:39
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.

Implement group by for lists
2 participants