typescript src/room
#437
typescript src/room
#437
Conversation
toger5
left a comment
toger5
left a comment
There was a problem hiding this comment.
Comments on things that I am not too confident:
|
|
||
| useEffect(() => { | ||
| setState({ json: state }); | ||
| setState(state); |
There was a problem hiding this comment.
why was it done with the {json: ...} before? Should the useState be changed to:
useState({json: {}} as {json: State});
There was a problem hiding this comment.
Yeah, not sure - would have to ask @robertlong, although I'd probably just remove the json
src src/settings src/auth
dbkr
left a comment
dbkr
left a comment
There was a problem hiding this comment.
Generally looks good on a first pass! On adding deps, just yarn add & commit package.json & yarn.lock as normal.
|
|
||
| useEffect(() => { | ||
| setState({ json: state }); | ||
| setState(state); |
There was a problem hiding this comment.
Yeah, not sure - would have to ask @robertlong, although I'd probably just remove the json
(this depends on: matrix-org/matrix-js-sdk#2527)
dbkr
left a comment
dbkr
left a comment
There was a problem hiding this comment.
This feels very nearly there - this will be most of the app done once its merged!
src/room/GroupCallInspector.tsx
Outdated
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const [context, _] = useState< | ||
| [State, React.Dispatch<React.SetStateAction<State>>] | ||
| >([{}, () => {}]); |
There was a problem hiding this comment.
Did you work out what was going on here? How does the context get set, and what's the weird value provided to the initialiser?
There was a problem hiding this comment.
I added a comment where the context gets set. I also fixed the initial value.
dbkr
left a comment
dbkr
left a comment
There was a problem hiding this comment.
Looks good! Thanks for adding the comment.
2 participants
For reviewers:
@types/react-router-domare there any conventions I have to follow for adding packages?Depends on:
Add missing events from reemitter to GroupCall matrix-org/matrix-js-sdk#2527