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

TNO-1193 Replace react-table component #663

Merged
merged 1 commit into from
Apr 14, 2023
Merged

TNO-1193 Replace react-table component #663

merged 1 commit into from
Apr 14, 2023

Conversation

Fosol
Copy link
Collaborator

@Fosol Fosol commented Apr 14, 2023

Created a new React table component that we can use to replace the react-table library. Ran into too many issues with react-table to want to continue with it. There are still pages using the react-table, but I'll remove them later.

The new component also has a headless design, but I've created a basic component that has the features we're using. It needs a lot more time to make it truly performant, but so far doesn't seem to be slower than react-table. There are some state management issues that do require a hard refresh when testing regrettably.

Published tno-core:0.0.232

You'll need to run make refresh n=editor to setup your local environment

Content List View

image

Morning Papers

image

@Fosol Fosol added bug Something isn't working enhancement New feature or request labels Apr 14, 2023
@Fosol Fosol self-assigned this Apr 14, 2023
@Fosol Fosol requested a review from jdtoombs as a code owner April 14, 2023 19:18
import React from 'react';
import { Button, FlexboxTable, ITableHookColumn, Row, Text } from 'tno-core';

export class DemoData {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Helpful page to test stuff... I'll leave it in for now.

import { ITableProps, SortFlag, TablePager, useTable } from '.';
import * as styled from './styled';

export const FlexboxTable = <T extends object>({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a default implementation. We can create other implementations because it's headless.

columns,
data,
options: {
isMulti: rest.isMulti,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd like to find a better way to do this as currently it require manually passing the values down.

</div>
</div>
)}
{table.options.showHeader && (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd like to extend the functionality of the hook to include ways to fetch all the attributes similar to react-table. It would make it easier to create new implementations.

</header>
)}
<div className="rows">
{table.groupBy && (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Basic grouping support. But it doesn't support paging when grouping.

* @param param0 Hook properties
* @returns A TableInternal object instance.
*/
export const useTable = <T extends object>({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not a fan of how I implemented state management with this hook. It seems to work okay for now though.

) => {
const cols = [...columns];
if (table.options.isMulti)
cols.unshift(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Multi-select comes built in.

Replace react-table
Publish tno-core:0.0.232
@Fosol Fosol merged commit b58d6b9 into bcgov:dev Apr 14, 2023
@Fosol Fosol deleted the tno-1193 branch April 14, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant