Skip to content
This repository has been archived by the owner on Dec 27, 2023. It is now read-only.

Add the searchEnabled flag to getSearchAction #27

Closed
wants to merge 2 commits into from
Closed

Add the searchEnabled flag to getSearchAction #27

wants to merge 2 commits into from

Conversation

dineshba
Copy link
Contributor

This is to fix the issue #25.

Please have a look and provide the feedbacks. Thanks.

@dineshba
Copy link
Contributor Author

I have added an example to test it. Please refer https://github.com/dineshba/flutter-search-bar/commit/c1fee7a0aa646b4887207e74d9b6af2eee9a605b

@dineshba
Copy link
Contributor Author

dineshba commented Oct 4, 2018

@ArcticZeroo When you are available please have a look at it

@ArcticZeroo
Copy link
Owner

I think we may want to implement it a different way. Here's my thoughts on the implementation:

  • We add another ValueNotifier for search being enabled (ideally a private one which is controlled by a getter and setter property)
  • The SearchAction is a widget whose opacity is controlled by the value of the value notifier

When the developer changes the value of the value notifier, the opacity can be updated pretty easily (though it may require wrapping the component). It seems heavy-handed to me to require the developer to know if the search is enabled when requesting the search action.

Realistically though this may be a failure of how I wrote the search bar in the first place, and I intend to eventually replace it with a single stateful widget.

Thoughts?

@dineshba dineshba closed this Jan 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants