Skip to content
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

Tidy up GiftedChat component #2379

Merged
merged 14 commits into from
May 5, 2023
Merged

Tidy up GiftedChat component #2379

merged 14 commits into from
May 5, 2023

Conversation

amerikan
Copy link
Contributor

@amerikan amerikan commented May 2, 2023

Purpose

  • adds Ref suffix to useRef variables for readability
  • consolidates state setter when mounted into a single setter
  • removes superfluous helper functions that are no longer needed

Unused props are not removed in this PR. Not sure why they're there, but thought I just leave them alone for now. Removed unused props.

@amerikan amerikan changed the title Tidy up GiftedChat component, resolves problem causing too many re-renders Tidy up GiftedChat component May 3, 2023
@@ -447,7 +376,7 @@ function GiftedChat(props: GiftedChatProps) {
const getBasicMessagesContainerHeight = (
composerHeight = state.composerHeight,
) => {
return getMaxHeight()! - calculateInputToolbarHeight(composerHeight!)
return maxHeightRef.current! - calculateInputToolbarHeight(composerHeight!)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

potential for NaN, as max height could be undefined.

@Johan-dutoit
Copy link
Collaborator

Johan-dutoit commented May 3, 2023

Otherwise LGTM. I've made some additional changes on your branch. You'll also notice the errors I was referring to in the other comments within the checks section.

@amerikan
Copy link
Contributor Author

amerikan commented May 3, 2023

@Johan-dutoit fixed the type errors, but don't think the Github actions is set up correctly.

@Johan-dutoit
Copy link
Collaborator

Johan-dutoit commented May 4, 2023

@Johan-dutoit fixed the type errors, but don't think the Github actions is set up correctly.

@amerikan my bad, was "working" on the PR. Fixed now and also added node 18. Will get the Circle CI removed soon.

@Johan-dutoit Johan-dutoit merged commit 1bec3d8 into FaridSafi:master May 5, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants