esentis / multiple_search_selection

A highly customizable multiple selection widget with fuzzy search functionality.
https://pub.dev/packages/multiple_search_selection
BSD 3-Clause "New" or "Revised" License
12 stars 13 forks source link

LateInitializationError on controller method #48

Closed anqit closed 7 months ago

anqit commented 7 months ago

Is your feature request related to a problem?

I'm having a couple of issues with the latest update that removes search field options and instead requires an entire TextField instead. I like the thought of adding more flexibility to the field, however this seems a bit heavy handed to me. To try to get this to work, I've basically had to dig through the older version of the code and copy and paste the text field that was previously being built. My first issue with this is that now, instead of having to provide just a few options that were of interest to me, I now have to provide the entire TextField. Again, I appreciate the flexibility, but now there is no ability to rely on default options that made things easier to use. Second, I'm trying to recreate the previously available behavior of being able to clear the search field with showClearSearchFieldButton. It used to be you could just set that to true and everything would work as expected. However, now you have to create the IconButton yourself and use the MultipleSearchSelectionController to clear the search field. But, it turns out that the controller's function isn't set until you initialize the MultipleSearchSelection itself, so this seems like a circular dependency to me. Here's basically what I am trying to do (irrelevant code is deleted):

  final _textController = TextEditingController();
  final _searchController = MultipleSearchController<User>();
// ... other stuff

Widget _selectUsersDropdown() =>
      MultipleSearchSelection<User>(
        controller: _searchController,
// ... other config
        searchField: _searchField(_searchController, _textController),
        hintText: widget.hintText, // slightly confused why this field still exists, shouldn't it just be what's on the search field?
      );

  TextField _searchField(MultipleSearchController<User> searchController, TextEditingController? textEditingController) =>
// this text field is mostly copied from the older version of this library
      TextField(
        enableSuggestions: true,
        autocorrect: true,
        controller: textEditingController,
        decoration: InputDecoration(
          contentPadding: const EdgeInsets.only(
            left: 6,
          ),
          hintText: widget._hintText ?? 'Search for users',
          hintStyle: const TextStyle(
            fontSize: 14,
            fontWeight: FontWeight.bold,
          ),
          border: OutlineInputBorder(
            borderSide: BorderSide.none,
            borderRadius: BorderRadius.circular(20),
          ),
          suffixIcon: IconButton(
            onPressed: searchController.clearSearchField, // **** HERE I try to recreate the clear search field button as before, but this function is not initialized
            icon: const Icon(
              Icons.clear,
            ),
          ),
        ),
      );

I guess the point of this issue is to see if you'd be willing to have it so you can provide the options individually as before, or if you wanted to provide the entire text field, to be able to do that as well, and default to the text field if it is provided, otherwise do the previous behavior. I am also curious to know how to recreate the clear search field button when the controller function has not been initialized.

Thanks!

esentis commented 7 months ago

Hello @anqit and thanks for pointing out the issue ! The real issue here is that the MultipleSearchSelectionController throws LateInitializationError which is obviously not intended.

Thanks and sorry for any incovenience that this may have caused ✌️

esentis commented 7 months ago

@anqit Can you try changing how you call the method ?

              suffixIcon: IconButton(
                onPressed: () {
                  controller.clearSearchField();
                }, //  Change here
                icon: const Icon(
                  Icons.clear,
                ),
              ),

Because the old way was calling the method while the widget was not yet built.

anqit commented 7 months ago

@anqit Can you try changing how you call the method ?

              suffixIcon: IconButton(
                onPressed: () {
                  controller.clearSearchField();
                }, //  Change here
                icon: const Icon(
                  Icons.clear,
                ),
              ),

Because the old way was calling the method while the widget was not yet built.

I'm guessing that would work, but I don't think that should be necessary. What if instead of each of the controller's methods being late initialized, the controller was assigned a late initialized reference to the search selection when the box is created? I think it would fix this problem, and semantically be more correct, because calling the methods would fail if the controller is never assigned. Something like:

class MultipleSearchSelectionController {
    late final MultipleSearchSelection selectionWidget;

    void setWidget(MultipleSearchSelection selectionWidget) { this.selectionWidget = selectionWidget; }

    void clearSelection() { selectionWidget.clearSelection(); }
    // .... other hooks
}

and then in the MultipleSearchSelection constructor:

MultipleSearchSelection._(
    // others
    MultipleSearchSelectionController? controller,
) controller = controller ?? MultipleSearchSelectionController() {
    controller.setWidget(this);
}

syntax might be off, but something like that?

anqit commented 7 months ago

Also, and I can open a separate issue if you think that would be better, but any thoughts to bringing back the other properties and having a default TextField?

esentis commented 7 months ago

I'll look into your suggestion if it could work, I haven't seen similiar pattern again to be honest.

Also, and I can open a separate issue if you think that would be better, but any thoughts to bringing back the other properties and having a default TextField?

Yes please do if it is not a big deal for you.

esentis commented 7 months ago

@anqit I have refactored the MultipleSearchController. You will no longer get the LateInitializationErrors with version 2.6.2 🥂

anqit commented 4 months ago

I tried upgrading again, but now getting The Scrollbar's ScrollController has no ScrollPosition attached when trying to scroll the shown items. I can open a separate issue, but curious if you or anyone else has noticed that?

esentis commented 4 months ago

@anqit can you please give me a minimal reproducable code ??

anqit commented 4 months ago

Yeah definitely, I'll create a new issue with that.