Refactor/optimize page loading #507
Conversation
remove correlated queries for opportunities, add indices
|
… end framework patterns
| } | ||
|
|
||
| // Helper function to scroll the virtualized table container to top | ||
| function scrollContainerToTop(): void { |
There was a problem hiding this comment.
See component_.cmd.scrollTo
There was a problem hiding this comment.
component_.cmd.scrollTo scrolls the document, but in this component we have a scrollable component. I have added a separate command called cmd.scrollContainerTo to cover this case.
src/front-end/typescript/lib/pages/user/lib/components/virtualized-table.tsx
Outdated
Show resolved
Hide resolved
| state.visibleRange.end | ||
| ); | ||
| const bodyRows = generateBodyRows(visibleUsersSlice); | ||
| const TableView = Table.view; |
There was a problem hiding this comment.
Using the built-in Table feels redundant here, given that we're only using it for its header. Theoretically, if someone were to use tooltips or add new messages for it in the future, the updates would not fire.
There was a problem hiding this comment.
I was able to eliminate the use of a table just for the purposes of the header, but the fix was fairly involved and required modifying the table body with custom markup to embed scrollable container inside the table: 787b91e
There was a problem hiding this comment.
I don't know about this component; I get the use case, but it doesn't follow the established state management of the app. I think it could work with some tweaks, i.e. abstracting it out into its own component like Table, but it will probably be complicated. If you want to try that go for it, but an alternative could be pagination which is already supported in the app.
There was a problem hiding this comment.
I created a separate component, similar to Table which can now be re-used in other parts of the application.
…se of displaying the header
src/front-end/typescript/lib/components/virtualized-table/index.tsx
Outdated
Show resolved
Hide resolved
| state | ||
| .set("scrollTop", 0) | ||
| .set("visibleRange", { start: 0, end: initialEnd }), | ||
| [component_.cmd.delayedDispatch(10, adt("scrollToTop", null))] |
There was a problem hiding this comment.
This seems to be doing nearly the same thing as init; will the component still work if we add this command to init an call init wherever we're dispatching nested resetScroll messages? i.e., when handling onInitResponse and search messages in src/front-end/typescript/lib/pages/user/list.tsx
There was a problem hiding this comment.
Refactored the code to eliminate "resetScroll" event and replaced it with calls to the init function, that now returns a command to scroll to the top of the component.
|
3 participants
This PR closes issue: [DM-42]
Includes tests? [N]
Updated docs? [N]
Proposed changes: