-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[Table] Add renderMode prop #1503
Conversation
Fix lintPreview: documentation | table |
packages/table/preview/index.tsx
Outdated
@@ -42,6 +42,7 @@ ReactDOM.render(<Nav selected="perf" />, document.getElementById("nav")); | |||
|
|||
import { IFocusedCellCoordinates } from "../src/common/cell"; | |||
import { IColumnIndices, IRowIndices } from "../src/common/grid"; | |||
import { RenderOptimizationMode } from "../src/common/renderOptimizationMode"; |
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.
Can we just call it render mode? And mention that we do optimizations in the docs? That way we can get rid of the extra mouthful.
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.
Sure
Rename to renderModePreview: documentation | table |
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.
Addresses #1374, #1415, #1495
Checklist
Changes proposed in this pull request:
Table
now supports arenderMode?: RenderMode
prop:RenderMode.BATCH
(default): renders cells in batches to improve performanceRenderMode.NONE
: renders cells synchronously all at onceTable
example:Reviewers should focus on:
I designed the prop as a
RenderMode
enum instead of auseBatchRendering
boolean to allow us to support additional rendering modes in the future. See: #1374 (comment), pasted here for your convenience: