Tables

[jstavh]

Add click-ability feature to certain whole rows (main / proposal tables and not secondary / votes tables).

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
mina-on-chain-voting-web	✅ Ready (Inspect)	Visit Preview	Sep 20, 2023 7:47pm

[jstavh]

This works. Just have to appease the testing code.

[jstavh]

@52 Requested Max's review. next/link work won't because it is in a table. Using <a> with<Link> is legacy methods. Resorted to useRouter to allow rows to be clickable. We used useRouter in our previous frontend technologies. This succeeds in the proposal (main) table but continues into the votes (pages) tables.

[52]

Couple additional points:

• Delete the saving-test directory and move the tests back. (No need to move any files around)

• Since you've made a change to the core component, the votes-table will now also have clickable rows, that routes to someplace wrong. This would also need to be addressed.

[jstavh]

Couple additional points:

• Delete the saving-test directory and move the tests back. (No need to move any files around)
• Since you've made a change to the core component, the votes-table will now also have clickable rows, that route to someplace wrong. This would also need to be addressed.

@52 I've tried your suggestions already but will revisit it.

[52]

Couple additional points:

• Delete the saving-test directory and move the tests back. (No need to move any files around)
• Since you've made a change to the core component, the votes-table will now also have clickable rows, that route to someplace wrong. This would also need to be addressed.

@52 I've tried your suggestions already but implemented them again in the most recent commit but getting errors, such as Property 'id' does not exist on type 'T'.ts(2339)

Yes, you're accessing a property on a generic type, TS cannot guarantee that it exists.
You need to modify the types to narrow it down.

As for the issue of the "click-ability" being shared between votes & proposal tables:
You'll have to do a couple things to make this work:

1. Create an additional prop called "variant" thats of type DataTableVariant

...
export const DataTableVariants = ['proposal', 'vote'] as const;
export type DataTableVariant = (typeof DataTableVariants)[number];

2. Pass the correct variant as props at each usage site. (ProposalTable & VotesTable)
3. Inside the core component, conditionally render the TableRow depending on which variant is passed in.

On the right track, though! 👍

[jstavh]

Couple additional points:

• Delete the saving-test directory and move the tests back. (No need to move any files around)
• Since you've made a change to the core component, the votes-table will now also have clickable rows, that route to someplace wrong. This would also need to be addressed.

@52 I've tried your suggestions already but implemented them again in the most recent commit but getting errors, such as Property 'id' does not exist on type 'T'.ts(2339)

Yes, you're accessing a property on a generic type, TS cannot guarantee that it exists. You need to modify the types to narrow it down.

As for the issue of the "click-ability" being shared between votes & proposal tables: You'll have to do a couple things to make this work:

1. Create an additional prop called "variant" thats of type DataTableVariant

...
export const DataTableVariants = ['proposal', 'vote'] as const;
export type DataTableVariant = (typeof DataTableVariants)[number];

2. Pass the correct variant as props at each usage site. (ProposalTable & VotesTable)
3. Inside the core component, conditionally render the TableRow depending on which variant is passed in.

On the right track, though! 👍

@52 Thanks, Max! I should've stuck with this path longer. Appreciate your notes.

[jstavh]

Couple additional points:

• Delete the saving-test directory and move the tests back. (No need to move any files around)
• Since you've made a change to the core component, the votes-table will now also have clickable rows, that route to someplace wrong. This would also need to be addressed.

@52 I've tried your suggestions already but implemented them again in the most recent commit but getting errors, such as Property 'id' does not exist on type 'T'.ts(2339)

Yes, you're accessing a property on a generic type, TS cannot guarantee that it exists. You need to modify the types to narrow it down.
As for the issue of the "click-ability" being shared between votes & proposal tables: You'll have to do a couple things to make this work:

1. Create an additional prop called "variant" thats of type DataTableVariant

...
export const DataTableVariants = ['proposal', 'vote'] as const;
export type DataTableVariant = (typeof DataTableVariants)[number];

2. Pass the correct variant as props at each usage site. (ProposalTable & VotesTable)
3. Inside the core component, conditionally render the TableRow depending on which variant is passed in.

On the right track, though! 👍

@52 Thanks, Max! I should've stuck with this path longer. Appreciate your notes.

@52 the prop isn't simply passed to the VotesTable. Adding const votesWithIds = votes.map((vote, index) => ({ ...vote, id: index })); fixes the data attribute but then the columns attribute columns similarly but less easily fixed unless we add an 'id' is one of the accessor keys, which I'm unsure yet if it is a good idea.

[jstavh]

Couple additional points:

• Delete the saving-test directory and move the tests back. (No need to move any files around)
• Since you've made a change to the core component, the votes-table will now also have clickable rows, that route to someplace wrong. This would also need to be addressed.

@52 I've tried your suggestions already but implemented them again in the most recent commit but getting errors, such as Property 'id' does not exist on type 'T'.ts(2339)

Yes, you're accessing a property on a generic type, TS cannot guarantee that it exists. You need to modify the types to narrow it down.
As for the issue of the "click-ability" being shared between votes & proposal tables: You'll have to do a couple things to make this work:

1. Create an additional prop called "variant" thats of type DataTableVariant

...
export const DataTableVariants = ['proposal', 'vote'] as const;
export type DataTableVariant = (typeof DataTableVariants)[number];

2. Pass the correct variant as props at each usage site. (ProposalTable & VotesTable)
3. Inside the core component, conditionally render the TableRow depending on which variant is passed in.

On the right track, though! 👍

@52 Thanks, Max! I should've stuck with this path longer. Appreciate your notes.

@52 the prop isn't simply passed to the VotesTable. Adding const votesWithIds = votes.map((vote, index) => ({ ...vote, id: index })); fixes the data attribute but then the columns attribute columns similarly but less easily fixed unless we add an 'id' is one of the accessor keys, which I'm unsure yet if it is a good idea.

@52 Solved :) by matching the expected type of the columns prop in the DataTable component to the columns being passed when using the VotesTable component.

[52]

Also, the following request is still needed:

• Delete the saving-test directory and move the tests back.

[jstavh]

@52 the code works, so can you elaborate on your edit revisions? The only thing that "does not work" are two tests, which look like they need more customization support beyond the fix we just deployed for Next 13's lack of support for next-router-mock (though supported by Next 11) and only released publicly scottrippey/next-router-mock#105 two (2) days ago. As you wrote here #289 -- given this is just to add click-ability to rows, can we move forward from here?

[52]

@52 the code works, so can you elaborate on your edit revisions? The only thing that "does not work" are two tests, which look like they need more customization support beyond the fix we just deployed for Next 13's lack of support for next-router-mock (though supported by Next 11) and only released publicly scottrippey/next-router-mock#105 two (2) days ago. As you wrote here #289 -- given this is just to add click-ability to rows, can we move forward from here?

Resolved offline, closing in favor of another approach.