MultiSelect

[Feichtmeier]

@jpnurmi any veto? :D

[Feichtmeier]

ehhh wait a sec, need to get the value out again 🤦

[Feichtmeier]

oook now it works :P

[jpnurmi]

Does it not look a bit old-fashioned? 👴 What comes to the implementation, take a look at how CheckedPopupMenuItem is implemented. :)

[Feichtmeier]

Well, at least in gnome we had this idea to make those kind of check lists monochrome

What's wrong with the implementation?

[jpnurmi]

When implemented as a PopupMenuItem, it can be a normal checkable item that is not limited to multi-selection and doesn't need to be wrapped into a dummy PopupMenuItem.

[Feichtmeier]

Alright Mr. fancy pans :P
WDYT of this instead?

fancy.webm

(I try to implement the interface you mentioned)

[jpnurmi]

Here's the Flutter implementation for benchmarking. Ideally, we'd only have some Yaru-prefixes but otherwise the same. :)

PopupMenuButton<MyEnum>(
  itemBuilder: (context) {
    return [
      for (final value in MyEnum.values)
        CheckedPopupMenuItem<MyEnum>(
          value: value,
          checked: enumSet.contains(value),
          child: Text(value.name),
        ),
    ];
  },
  onSelected: (value) {
    setState(() {
      if (enumSet.contains(value)) {
        enumSet.remove(value);
      } else {
        enumSet.add(value);
      }
    });
  },
)

[Feichtmeier]

2022-10-21.18-37-27.mp4

The material component has the advantage of just being there but our implementation has the advantage of letting the popup stay open which is becomes much better if you think of many filters like for example packagekitfilter

[jpnurmi]

i know multi-selection is the ultimate goal here but let's take it step by step and focus on the basic checkable popup menu item first. :) multi-selection is kind of a specialized variant of that. we can build it next once we have the basic checkable popup menu item to build upon.

[Feichtmeier]

I use a in-app widget for now https://github.com/ubuntu-flutter-community/software/pull/422

[Feichtmeier]

@jpnurmi implemented your change wishes

Can you tackle the do-not-close-on-selection-problem after this is merged?

[jpnurmi]

YaruCheckedPopupMenuItem

class YaruCheckedPopupMenuItem<T> extends PopupMenuItem<T> {
  const YaruCheckedPopupMenuItem({
    super.key,
    super.value,
    this.checked = false,
    super.enabled,
    super.padding,
    super.height,
    super.mouseCursor,
    super.child,
  });

  final bool checked;

  @override
  PopupMenuItemState<T, YaruCheckedPopupMenuItem<T>> createState() =>
      _YaruCheckedPopupMenuItemState<T>();
}

class _YaruCheckedPopupMenuItemState<T>
    extends PopupMenuItemState<T, YaruCheckedPopupMenuItem<T>> {
  @override
  Widget buildChild() {
    return IgnorePointer(
      child: ListTile(
        enabled: widget.enabled,
        leading: _YaruCheckIndicator(checked: widget.checked),
        title: widget.child,
      ),
    );
  }
}

The usage is identical to Flutter:

YaruPopupMenuButton<MyEnum>(
  itemBuilder: (context) {
    return [
      YaruCheckedPopupMenuItem(
        ...
      ),
  },
  onSelected: (value) {
    ...
  },
)

yaru_checked_popup_menu_item.webm

YaruMultiSelectPopupMenuItem

class YaruMultiSelectPopupMenuItem<T> extends PopupMenuItem<T> {
  const YaruMultiSelectPopupMenuItem({
    super.key,
    super.value,
    this.checked = false,
    super.enabled,
    super.padding,
    super.height,
    super.mouseCursor,
    super.child,
    this.onChanged,
  });

  final bool checked;
  final ValueChanged<bool>? onChanged;

  @override
  PopupMenuItemState<T, YaruMultiSelectPopupMenuItem<T>> createState() =>
      _YaruMultiSelectPopupMenuItemState<T>();
}

class _YaruMultiSelectPopupMenuItemState<T>
    extends PopupMenuItemState<T, YaruMultiSelectPopupMenuItem<T>> {
  var _checked = false;

  @override
  void handleTap() {
    setState(() => _checked = !_checked);
    widget.onChanged?.call(_checked);
  }

  @override
  void initState() {
    super.initState();
    _checked = widget.checked;
  }

  @override
  Widget buildChild() {
    return IgnorePointer(
      child: ListTile(
        enabled: widget.enabled,
        leading: _YaruCheckIndicator(checked: _checked),
        title: widget.child,
      ),
    );
  }
}

In this case, one is no longer selecting a single item so the handler needs to be per-item but something like this is as close as it gets :)

YaruPopupMenuButton<MyEnum>(
  itemBuilder: (context) {
    return [
      YaruMultiSelectPopupMenuItem(
        ...
        onChanged: (value) {
          ...
        },
      ),
  },
)

yaru_multi_select_popup_menu_item.webm

[jpnurmi]

Sorry I was busy most of yesterday but I was just looking into it last night and continued in the morning. :)

[Feichtmeier]

np, yeah per-item on-tap was also how I did it in my custom solution
SHall I add your changes to this PR or in another PR?

[jpnurmi]

Is there anything we could do about the alignment? It looks somehow unbalanced :)

[jpnurmi]

what about minLeadingWidth: 18? (the size of the check indicator)

[Feichtmeier]

minLeadingWidth: 18

done

what about this?

SHall I add your changes to this PR or in another PR?

[jpnurmi]

LGTM 👍

[Feichtmeier]

@jpnurmi will you add the multi select thing? Or shall I copy your code?

[jpnurmi]

Sure, feel free to. 🙂 Does it work as expected?