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

Suggestions list behavior improved #79

Merged
merged 3 commits into from
Apr 7, 2025

Conversation

luis901101
Copy link
Contributor

  • Fixes suggestions overlapping with welcome message by handling suggestions visibility depending on the TextField focus.
  • Adds scroll to suggestions

Issues fixed by this PR:

Demos:

Welcome.message.with.Suggestions.mp4
Welcome.message.with.a.lot.of.Suggestions.mp4

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read the Flutter Style Guide recently, and have followed its advice.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@csells
Copy link
Contributor

csells commented Apr 4, 2025

@luis901101 can you explain your changes to me? you seem to have made a number of them that don't have anything to do with overlapping suggestions, e.g. pulling the geminiApiKey from the environment. can you provide a minimal PR that only addresses the issue at hand?

@luis901101
Copy link
Contributor Author

@luis901101 can you explain your changes to me? you seem to have made a number of them that don't have anything to do with overlapping suggestions, e.g. pulling the geminiApiKey from the environment. can you provide a minimal PR that only addresses the issue at hand?

  • gemini_api_key.dart: Sorry I missed that change, it was only for my local env to avoid setting my gemini api key directly in the code, I will revert it.
  • chat_input.dart: Here was necessary to add a property to set the autofocus so in case there are suggestions the input should not be focused by default.
  • llm_chat_view.dart: WidgetsBindingObserver was added to know every time the keyboard is showing to show/hide the suggestions. ChatSuggestionsView now will animatedly show/hide and will also be scrollable, also it's the child of a Container with a solid background color to make sure that everything behind the suggestion list get completely hidden, this is useful when the suggestions list is long and there is a welcome message.
  • chat_suggestion_view.dart: Wrap alignment was set to center because the Align widget was replaced by a Container with a background color in LlmChatView, that Container could set the alignment property to center but that will cause the suggestions to fill the whole available space making the welcome message invisible due to the solid color of Container, so by setting the alignment property to the Wrap widget, the content will not expand to fill the available space thus the suggestions height will be only the necessary.
  • suggestions_test.dart: I added a few tests to check the behavior of suggestions when no suggestions, a few suggestions or a lot of suggestions are set, with a welcome message and when the keyboard is shown/hidden.

@luis901101
Copy link
Contributor Author

@csells from our last conversation here and here I just pushed some changes to achieve why we agreed.

Basically now the Suggestions will be included in the ListView with the WelcomeMessage if any, this way as you mentioned the scrolling and overlap will be solved.

Just one note: the suggestions will be shown below the welcome message instead of above. I think it makes more sense for the suggestions to appear right after the welcome message in the ListView. In fact, once the user taps on a suggestion and sends the message, from the user’s point of view it feels like the suggestion list is replaced by the selected suggestion which, again, I think makes sense.

Let me know if anything is not clear or should be different.

Here some demos:

Welcome.message.with.Suggestions.inside.ListView.mp4
Welcome.message.with.a.lot.of.Suggestions.inside.ListView.mp4

@csells
Copy link
Contributor

csells commented Apr 7, 2025

well done. I think that solves the problems nicely.

@csells csells merged commit 02158e6 into flutter:main Apr 7, 2025
1 check 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.

2 participants