[FTF-491] Add a nested table component #3130
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
m-hulbert
force-pushed
the
nested-table-component
branch
from
8ff275f to
e0822dc
Compare
m-hulbert
force-pushed
the
nested-table-component
branch
from
e0822dc to
bb2a54b
Compare
aralovelace
left a comment
aralovelace
left a comment
There was a problem hiding this comment.
hi @m-hulbert this is great just a few small feedback
|
|
||
| export const NestedTableExpandButton: React.FC<NestedTableExpandButtonProps> = ({ typeName, expanded, onClick }) => { | ||
| return ( | ||
| <button |
There was a problem hiding this comment.
nitpick we might need to add ARIA attributes for screen readers
| useEffect(() => { | ||
| if (id && parsedProperties.length > 0) { | ||
| const tableData: TableData = { | ||
| id, | ||
| properties: parsedProperties, | ||
| }; | ||
| register(id, tableData); | ||
| } | ||
| }, [id, parsedProperties, register]); |
There was a problem hiding this comment.
The useEffect that registers tables doesn't unregister on unmount, which can leave stale entries.
Suggested:
// In NestedTableContext.tsx - add unregister function
const unregister = useCallback((id: string) => {
if (registryRef.current.has(id)) {
registryRef.current.delete(id);
setRegistryVersion((v) => v + 1);
}
}, []);
// In NestedTable.tsx - add cleanup
useEffect(() => {
if (id && parsedProperties.length > 0) {
const tableData: TableData = { id, properties: parsedProperties };
register(id, tableData);
return () => {
unregister(id);
};
}
}, [id, parsedProperties, register, unregister]);
| const descriptionCell = cells[2] as ReactElement<{ children?: ReactNode }>; | ||
| const typeCell = cells[3] as ReactElement<{ children?: ReactNode }>; | ||
|
|
||
| const name = extractText(nameCell.props.children); |
There was a problem hiding this comment.
might need to add null check?
| @@ -0,0 +1,93 @@ | |||
| import React, { useEffect, useMemo } from 'react'; | |||
There was a problem hiding this comment.
We might error boundary for parsing failures? what you think? @kennethkalmer ?
Description
This PR adds a new nested table component described here: FTF-491.
It essentially allows for tables listing properties to be nested within one another when they reference a custom type, or enum.
This PR has several parts:
<Table>tags with various variables controlling this. Usingidensures the table can be referenced in another table cell, withhiddenensuring it isn't rendered standalone in the output.hiddenproperty is removed from markdown when rendering pages available as raw/copied MD to ensure LLMs do not ignore the content.Checklist