-
Notifications
You must be signed in to change notification settings - Fork 6
Remote data operations samples update to 19 #781
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
Conversation
import React, { useEffect, useRef } from 'react'; | ||
import ReactDOM from 'react-dom/client'; | ||
import './index.css'; | ||
|
||
import { IgrForOfStateEventArgs, IgrGridModule } from 'igniteui-react-grids'; | ||
import { IgrForOfState } from 'igniteui-react-grids'; |
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.
React
andIgrForOfState
are unused
Edit: Strike that last, JSX silliness requires React
in scope
}); | ||
}, []); | ||
|
||
function handlePreLoad(e: IgrForOfStateEventArgs) { |
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.
and IgrForOfStateEventArgs
isn't imported, possibly typo on the unused IgrForOfState import? If so, you can change the import name
import "./index.css"; | ||
|
||
import { IgrGrid, IgrPaginator, IgrGridModule, GridPagingMode } from "igniteui-react-grids"; | ||
import { IgrGrid, IgrPaginator, GridPagingMode } from "igniteui-react-grids"; |
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.
GridPagingMode is unused
import ReactDOM from 'react-dom/client'; | ||
import './index.css'; | ||
|
||
import { IgrForOfStateEventArgs, IgrGridModule } from 'igniteui-react-grids'; | ||
import { IgrForOfState } from 'igniteui-react-grids'; | ||
import { IgrGrid, IgrColumn } from 'igniteui-react-grids'; | ||
import { loadDataForPage, getCachedData } from './NwindService'; | ||
|
||
import 'igniteui-react-grids/grids/combined'; |
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.
Oh forget this is also very much not needed, though will prob be mass-removed
const grid = useRef<IgrGrid>(null); | ||
const paginator = useRef<IgrPaginator>(null); |
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.
Also in general, refrain from manipulating state through refs - if you're seeing useRef
in the sample and it's not being used to call methods - suspect its usage entirely and see if it can be replaced with state and bindings instead.
I.e.:
paginator totalRecords can and should be state that's bound to the paginator, that's the expected usage pattern and it should work
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.
To avoid this sitting longer and more conflict resolutions, I've updated the imports (specifically one that'd error) so this can be merged. The "state instead of refs" comment is likely valid more broadly and can be addressed separately.
No description provided.