-
Notifications
You must be signed in to change notification settings - Fork 38
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
Made expandable content a separate component #2240
Conversation
packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx
Outdated
Show resolved
Hide resolved
packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx
Outdated
Show resolved
Hide resolved
packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx
Outdated
Show resolved
Hide resolved
…/iTwin/iTwinUI into uyen/separate-expandable-content
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.
LGTM!
packages/itwinui-react/src/core/Table/TableExpandableContentMemoized.tsx
Outdated
Show resolved
Hide resolved
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.
LGTM after the visual test is fixed. But since this PR is a part of #2208, we shouldn't merge this right away. You can update #2208 to point to this PR's branch instead of main. More info about stacking PRs.
I'm good with merging it tbh. No reason to leave it hanging. It's a small, standalone PR that does exactly what it says. |
Then merging it sounds good too in this case. If we need to make any changes related the contents of this PR, we can then do it in #2208 itself or in another PR. |
Changes
Separate expandable content in
Table
to another component, independent from the originalTableRowMemoized
. This PR prepares for the fixing of the bug that is caused by sub-component not being recognized by the virtualizer when being rendered within the main row.Testing
Confirmed the expandable contents visually turned out correctly.
Docs
N/A
After-PR TODO: