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

SuggestionBox can be on top of TextField. #20

Closed
wants to merge 3 commits into from

Conversation

Rassell
Copy link

@Rassell Rassell commented Jan 7, 2019

  • Finish conflicts from:
    Make SuggestionBox can be on top of TextField. #10

  • Added noResize flag in order to put suggestions on top (or opposite direction) if items collapse with the keyboard

  • If the list is on top change order of suggestion to have close the first ones

feat: flag to disable resize and put the opposite direction
@AbdulRahmanAlHamali
Copy link
Owner

Thank you for your contribution @Rassell , it is highly appreciated by everyone using this package and me!

I will review your contribution within a few days and hopefully merge it. But I have to ask, why are there many changes that don't seem relevant to the pull request?

@Rassell
Copy link
Author

Rassell commented Jan 8, 2019

Well, it's about when the suggestions are wanted to be shown but there is no space between them and the keyboard shown them on top.

@AbdulRahmanAlHamali
Copy link
Owner

I understand, and it is definitely a good idea. But looking at the commit, there are many things changed that are not related to the actual change that you made, which is not helping when trying to understand the detail of your pull request.

It is better to push a commit that changes only the parts immediately related to your change, otherwise it will be hard to review, and will make things harder to manage in the future.

Thank you again!

@KaYBlitZ
Copy link
Contributor

Added in #28

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.

3 participants