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

Updating the code to fix MinComposerHeight is not respecting props in some instances. #1078

Merged
merged 1 commit into from Jan 11, 2019

Conversation

Projects
None yet
2 participants
@nandiniparimi1107
Copy link
Contributor

nandiniparimi1107 commented Jan 11, 2019

Issue

minComposerHeight is hardcoded to use constant instead of props in few instances. Due to this minimumToolbarHeight will give incorrect result thus making UI look inconsistent.

Solution

updating the code to use the props in all instances.

Testing

if we add minComposerHeight prop to the giftedChatComponent, there is a bottomOffset on keyboard open as the minInputToolbarHeight is returning incorrect value based on the hardcoded minComposerHeight.

@nandiniparimi1107

This comment has been minimized.

Copy link
Contributor

nandiniparimi1107 commented Jan 11, 2019

@xcarpentier Please take a look at this.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 11, 2019

Codecov Report

Merging #1078 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1078   +/-   ##
======================================
  Coverage    43.1%   43.1%           
======================================
  Files          21      21           
  Lines         515     515           
  Branches      112     112           
======================================
  Hits          222     222           
  Misses        221     221           
  Partials       72      72
Impacted Files Coverage Δ
src/GiftedChat.js 24.53% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 837bbf2...16b8846. Read the comment docs.

1 similar comment
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 11, 2019

Codecov Report

Merging #1078 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1078   +/-   ##
======================================
  Coverage    43.1%   43.1%           
======================================
  Files          21      21           
  Lines         515     515           
  Branches      112     112           
======================================
  Hits          222     222           
  Misses        221     221           
  Partials       72      72
Impacted Files Coverage Δ
src/GiftedChat.js 24.53% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 837bbf2...16b8846. Read the comment docs.

@xcarpentier

This comment has been minimized.

Copy link
Collaborator

xcarpentier commented Jan 11, 2019

Ok I see.
I agree with your change. I will merge in master but have no time to test.
Please give your test feedback.

@xcarpentier xcarpentier merged commit f872d67 into FaridSafi:master Jan 11, 2019

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/project 43.1% remains the same compared to 837bbf2
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment