New variant for tags-mode #78

ringvold · 2024-08-16T20:15:36Z

Hi! 😄

This is a suggestion for a new variant of the tag mode. The multiple_select option for the new tags_mode attribute makes it possible to select multiple options quickly without needing to search or force the dropdown to appear. The current tag mode hides the dropdown-part when a option is selected. This approach could probably be described as a kind hybrid of the normal live_select tag mode and a normal multi select input.

A new attribute tags_mode is added with :default (how tags work today) and :multiple_select as possible options.

Example usage taken from demo-app:

<.live_select
   field={my_form[:city_search]}
   selected_option_class="cursor-pointer font-bold hover:bg-gray-400 rounded"
   mode={:tags}
   tags_mode={:multiple_select}
   placeholder="Search for a city"
   update_min_len={3} />

There is some code in the demo app related to selected_option_class as this feature would need some new style defaults/style changes to look passable out of the box. This should probably be solved some other way. Maybe by adding some new default styles.

The name chosen, multiple_select, is very much up for debate. I just needed something to work with.

Potential performance issue

When testing in the demo app I do notice some slowness when selecting or deselecting many options in rapid succession. I did not experience this in our app. I am not sure if this is a flaw in the implementation or just that the dataset for the demo app is very large. Any suggestions on this point is especially welcome.

Breaking changes

The feature also changes the value exposed to the option-slot to also return an indication if the option is already selected. This is one of the main points of the change as the impetus for this feature is a design/UX that displays the selected with a checkbox. See attached picture for example. I suppose this could be done with just styles but the design seems harder to replicate that way and I also think it would be nice to be able to have that info available to the slot. But still open for debate. :)

Example of option-slot usage:
Notice the :let now exposing a tuple which contains the option and a selected boolean.

<:option :let={{%{label: label, value: value}, selected}}>
  <div class="flex justify-content items-center">
    <input
      class="rounded w-4 h-4 mr-3 border border-border"
      type="checkbox"
      checked={selected}
    />
    <span class="text-sm"><%= label %></span>
  </div>
</:option>

Rendered example:

This is a suggestion based on functionality that we where in need of and I hope it can be added to the project in some form.

This is not expected to be merged as-is (ex. tests is missing for now) but it is working code that we would use in our fork and a good point to discuss from I think.

I'm aware that this might not be something that this project wants and I'm happy to make changes or discuss the approach. Maybe there is a better way to achive the same or something very similar?

The changes necessary does not seem to invasive though from my perspective (except maybe the option-slot breaking change 😅).

Let me know what you think. 😄

EDIT: I forgot while writing this, but there is also a bug in this implementation I have not figured out. When making the first selection the blur event triggers and hides the dropdown. This does not happen on other selections but consistently happens from state of nothing selected to first option selected.

maxmarcon · 2024-08-17T05:54:27Z

Hi @ringvold thanks a lot for this, I just took a superficial look but it sounds like solid work 💪 using checkboxes in the dropdown is something I've been thinking of adding for a long time now.

Unfortunately, I'm quite busy this coming week, but I will take a proper look as soon as I find some time. Cheers!

maxmarcon · 2024-08-29T19:35:17Z

Hi @ringvold I finally found the time to read carefully what you wrote (I've been really busy in the past few weeks, still busy but things are getting a bit better).

I haven't looked at the code yet. Here's a couple of comments:

I like the idea of enabling a "quick selection mode". I'm not sure I would add a new attribute for it though. To me, it feels cleaner and less clunky to have this as a third mode: :single, :tags and (name also totally up for debate), :quick_tags. Let me know what you think

EDIT: thinking about it a bit more, a simple additional boolean attribute :close_dropdown_on_select would also do and be more explicit.

Couldn't the performance issue you were observing be caused by latency? Were you trying out your app locally? Regardless, the demo app has a bit more of 800 cities, and the search is slow indeed. I should probably add an index, right now the search simply traverses the list of cities and collect the matches.
I really like the idea of exposing the selection status to the option slot. However, I don't think that this needs to be a breaking change. Instead of exposing a tuple, we can add a new field to the map:

<:option :let={%{label: label, value: value, selected: selected}}>

In this way, old code that ignores the selected field would still work

it would be great to allow the user to deselect options using the checkboxes in the selection. But not sure right now how to do that, it's just an idea and probably out of the scope of your feature. In any case, exposing the selected status to the slot is probably a good first step in this direction.

Let me know what you think

ringvold · 2024-09-03T12:32:57Z

Hi!

Thought I'd get back to you sooner but stuff happened here as well.

I am very open to how the public API should be. I always thought of it as a variant of the tag mode, but I see how it also could be just a configuration for the tag mode from a users perspective. I felt "quick selection" as you named it has a good description but as for how it should be exposed to the user I'm not sure.

Functionally it is a bit more than "don't close the dropdown" (as I made it here at least) as it also enables you to deselect from the drop down which I believe is not possible on "normal" tags mode. I think that might be why I think of it more as a variant of tag mode. You should have the final say on the API but I feel like :close_dropdown_on_select do not fully encapsulate the functionality I had in mind.

So I lean towards :quick_select/:quick_tags or something like that.

Yes, I tried it locally and still experienced slowness. I tested it out again and it is most pronounced when a lot of elements are selected and selecting very quickly. I suspect it might be a combination of processing a large list of options and handling many selections in quick succession. Maybe a debounce of the selection and handle an event with multiple selections solves it, but that sound like it might be a larger refactor.
Yeah, that should work. Is there any way this could interfere with some custom fields a user might have used with the same name?
I'm not sure what you mean by allowing to use checkboxes. I think about them as visual aide. Selection technically happens on interaction with the parent element. Are there any advantages to changing this? As in it could be advantageous to move the click handler to the checkbox? Exposing the selected status enables custom markdown for selected/unselected options as an addition to the css options available today.

Other things of note

I have noticed that this PR breaks the current behavior for tags as you can deselect from the drop down. Which you can not to in the current released version. I have tried to not change the current functionality of tag mode so I'm not sure what happened there. I consider that a bug.

One thing I have not figured out yet (although not look at it very much yet) is how to not loose focus of the element when selecting it when operating it with keyboard only. Currently it resets focus to the search box when selecting. I would like to keep it in the dropdown at/around the selected element to be able to continue the selection. Any thoughts on this?

maxmarcon · 2024-09-10T20:02:37Z

Functionally it is a bit more than "don't close the dropdown" (as I made it here at least) as it also enables you to deselect from the drop down which I believe is not possible on "normal" tags mode. I think that might be why I think of it more as a variant of tag mode. You should have the final say on the API but I feel like :close_dropdown_on_select do not fully encapsulate the functionality I had in mind.

Yeah apologies I hadn't realized you can deselect from the dropdown...

I'm not sure what you mean by allowing to use checkboxes

And this is exactly what I meant by "using checkboxes to deselect".

Ok so I agree with you now, close_dropdown_on_select obviously doesn't cut it. A third selection mode is more appropriate. quick_tags sounds good to me!

Yeah, that should work. Is there any way this could interfere with some custom fields a user might have used with the same name?

You mean custom fields they're passing to the slots? To preempt this, we could simply taise an exception when someone uses the selected key for the options.

I will take a look at the last 2 points you mentioned (breaking current behavior and not losing focus) soon.

But it sounds like at least we agreed on how to expose this new mode in the API and how to pass the selected status in the slots :)

ringvold · 2024-09-13T10:39:58Z

Yes, it does sound like we are onto something her 😄

You mean custom fields they're passing to the slots? To preempt this, we could simply raise an exception when someone uses the selected key for the options.

That is maybe also a breaking change but certainly much smaller than my suggestion.

I will make a suggestion on the default styles for this mode. I cant remember the details but there are some changes that need to be made to make it look decent out of the box.

maxmarcon · 2024-09-14T18:55:00Z

That is maybe also a breaking change but certainly much smaller than my suggestion.

I don't think this is a breaking change because we're not breaking any API. The docs say that the options can be specified as maps with the shape %{label: label, value: value}. It doesn't state that other keys in the map will be made available to the slots.

egeersoz · 2024-09-19T15:36:36Z

Is this one good to merge?

maxmarcon · 2024-09-19T19:47:18Z

@egeersoz no, we just discussed a few points and I'm sure @ringvold has still a few things to add before it's ready for prime time :)

ringvold · 2024-09-20T09:10:15Z

@egeersoz I still have to implement what me and Max talked about here, add test and then probably a new check by @maxmarcon. I hope to find some time to look at it this weekend though. :)

ringvold · 2024-10-03T11:47:56Z

Life happens and I have not gotten to this when I thought. 🕙🏃‍➡️

@maxmarcon Looking a bit at it now and I remember why I did this functionality as a new variant for tags. It is a bit easier to implement when it is just a variant of tags as we can piggyback on the existing tags functionality with some other minor changes. Introducing a new mode we have to make sure not to mess up :tags and also add :quick_tags to most, if not all places where we are checking for :tags. It will probably be fine but I think that is why I defaulted to a new variant for tags.

On another note my fix for the dropdown removal on first select did not work. It basically just removed all phx-focus use defined externally (parent focus events). 😅

The fix seems to be to always render the tags container div but conditionally render the rest. (It might be the re rendering when the first tag is added that triggers the blur event that hides the dropdown.) In practice it would be to move the if on line 11 inside the div beneath it.
The down side of this approach is that the dropdown is not closed when removing the last selected tag which I am now used to, but it probably cant be helped.

Hoping to have something to look at soon.

The multiple select option for the new tags_mode makes it possible to select multiple options at once without needing to search or force the dropdown to appear. It is a kind of hybrid of the search variant and a normal multi select input input.

Do not move focus back to search box on first select. Also refactor to use more efficient way of checking for already selected.

Adds toggle to showcase app to view example of custom html for options list. Also fixes issue with dropdown disappearing when selecting first time in quick_tags mode.

ringvold · 2024-12-12T16:24:30Z

Rebased on latest master. Fixed tests for tags and added tests for quick_tags.

lib/support/live_select_web/live/showcase_live.ex

ringvold · 2024-12-12T16:43:03Z

@maxmarcon Let me know if there are any more things that need to be fixed og changed. :)

When I think about it we might need to add some documentation. 🤔

egeersoz · 2024-12-18T16:33:04Z

@maxmarcon Can this be merged?

maxmarcon · 2024-12-26T11:31:54Z

@ringvold this looks awesome, thank you! Review coming soon.

FYI: I pushed a couple of minor commits to your branch just to test if I could do it (I updated a line in the docs).

maxmarcon

When I think about it we might need to add some documentation. 🤔

Don't worry, I will add it after this MR is merged. It's not going to be a lot of documentation anyway

lib/live_select/component.ex

maxmarcon · 2024-12-26T13:56:34Z

lib/live_select/component.ex

+  defp next_selectable(%{
+         options: options,
+         active_option: active_option,
+         mode: :quick_tags
+       }) do
+    options
+    |> Enum.with_index()
+    |> Enum.reject(fn {opt, _} -> active_option == opt end)
+    |> Enum.map(fn {_, idx} -> idx end)
+    |> Enum.find(active_option, &(&1 > active_option))
+  end


this is unnecessary code duplication IMO. All you need to do is change like 717 to something like:

|> Enum.reject(fn {opt, _} -> active_option == opt || (mode != :quick_tags && already_selected?(opt, selection)) end)

and you have a version that works for both :quick_tags and non-:quick_tags modes :)

maxmarcon · 2024-12-26T13:56:54Z

lib/live_select/component.ex

+  defp prev_selectable(%{
+         options: options,
+         active_option: active_option,
+         mode: :quick_tags
+       }) do
+    options
+    |> Enum.with_index()
+    |> Enum.reverse()
+    |> Enum.reject(fn {opt, _} -> active_option == opt end)
+    |> Enum.map(fn {_, idx} -> idx end)
+    |> Enum.find(active_option, &(&1 < active_option || active_option == -1))
+  end
+


this is unnecessary code duplication IMO. All you need to do is change like 746 to something like:

|> Enum.reject(fn {opt, _} -> active_option == opt || (mode != :quick_tags && already_selected?(opt, selection)) end)

and you have a version that works for both :quick_tags and non-:quick_tags modes :)

lib/live_select/component.ex

maxmarcon · 2024-12-26T14:02:17Z

lib/live_select/component.html.heex

-      class(@style, :tags_container, @tags_container_class, @tags_container_extra_class)
-    }>
+  <div class={class(@style, :tags_container, @tags_container_class, @tags_container_extra_class)}>
+    <%= if (@mode == :tags || @mode == :quick_tags) && Enum.any?(@selection) do %>


I prefer @mode in [:tags, :quick_tags] , I think it's more readable (nitpick, I know)

test/live_select/component_test.exs

maxmarcon

@ringvold, thanks again for the great work. The feature looks amazing and you also added the styling of the options as checkboxes as a bonus. Appreciated that you also changed the showcase app, thanks a ton!

I'm requesting a few changes (mostly minor code refactoring, and one change that will simplify how the default styles for quick_tags are set in the showcase app)

I pushed a modification to a test in your branch so that even the upwards navigation in the new quick_tags mode is covered...

Again, this is great stuff!!! Very eager to merge this one

maxmarcon · 2024-12-26T15:09:48Z

lib/support/live_select_web/live/showcase_live.ex

@@ -67,9 +67,15 @@ defmodule LiveSelectWeb.ShowcaseLive do
      field(:allow_clear, :boolean)
      field(:debounce, :integer, default: Component.default_opts()[:debounce])
      field(:disabled, :boolean)
+      field(:custom_option_html, :boolean)


I love that you went out of the way to create this toggle to show how to style the options as checkboxes!

Can we please rename it to something more specific like "Options styles as checkboxes" ?

maxmarcon · 2024-12-26T15:10:06Z

lib/support/live_select_web/live/showcase_live.html.heex

@@ -71,6 +71,10 @@
              <span class="label-text mr-1">Disabled:&nbsp;</span>
              {checkbox(@settings_form, :disabled, class: "toggle")}
            <% end %>
+            <%= label class: "label cursor-pointer" do %>
+              <span class="label-text mr-1">Custom option HTML:&nbsp;</span>
+              <%= checkbox(@settings_form, :custom_option_html, class: "toggle") %>


I love that you went out of the way to create this toggle to show how to style the options as checkboxes!

Can we please rename it to something more specific like "Options styles as checkboxes" ?

maxmarcon · 2024-12-26T15:27:04Z

lib/support/live_select_web/live/showcase_live.html.heex

@@ -326,7 +345,10 @@
            <.copy_to_clipboard_icon />
          </button>
        </div>
-        <Render.live_select opts={Settings.live_select_opts(@settings_form.data, true)} />
+        <Render.live_select
+          custom_option_html={@settings_form[:custom_option_html].value}


the argument of the function component should also be renamed to options_styled_as_checkboxes

lib/support/live_select_web/live/showcase_live.ex

maxmarcon · 2024-12-26T15:42:26Z

lib/support/live_select_web/live/showcase_live.ex

+        attrs =
+          if settings.mode == :quick_select do
+            %{selected_option_class: "cursor-pointer font-bold hover:bg-gray-400 rounded"}
+          else
+            %{}
+          end
+
        socket =
          socket
-          |> assign(:settings_form, Settings.changeset(settings, %{}) |> to_form)
+          |> assign(:settings_form, Settings.changeset(settings, attrs) |> to_form)


Suggestion: let's remove this code and all other changes in this file related to the selected_option_class override, and add this:

params = if params["mode"] == "quick_tags" do Map.put_new( params, "selected_option_class", "cursor-pointer font-bold hover:bg-gray-400 rounded" ) else params end

just before we create the changeset. Advantages:

simpler

more transparent, as the user will see the classes in the UI's styling options

the user can override these default classes if they want to

Yeah, what I had done was kind of a hack to just to make it work.
This is nice, but it seems to not reset when you change back to tags mode.
Maybe do a reset of selected_option_class in the else clause or just let it be?

I think just let it be, at least for now. I mean if the user changes the style while in quick_tags mode you don't want it to be reset when you change back to tags mode... the user will lose what they entered.

I guess for now this is the best we can do, we can come up with a better way later :)

ringvold · 2024-12-27T12:01:04Z

I think all comments should have been addressed now. Let me know if I missed something. :)

maxmarcon · 2024-12-27T13:13:38Z

🥳 🚀 💪

ringvold changed the title ~~Add variant for tags-mode~~ New variant for tags-mode Aug 16, 2024

ringvold force-pushed the multi-select-tags-mode branch from 691c311 to f4d4e24 Compare August 16, 2024 20:20

maxmarcon added the enhancement New feature or request label Aug 29, 2024

ringvold force-pushed the multi-select-tags-mode branch 2 times, most recently from 40e36ff to 666399a Compare October 4, 2024 09:36

ringvold added 3 commits December 12, 2024 10:55

Fix focus issue on first select

cded2de

Do not move focus back to search box on first select. Also refactor to use more efficient way of checking for already selected.

Change multi-select variant to quick_tags mode

3db95ff

Adds toggle to showcase app to view example of custom html for options list. Also fixes issue with dropdown disappearing when selecting first time in quick_tags mode.

ringvold force-pushed the multi-select-tags-mode branch 2 times, most recently from 4ad12d5 to 4717fbf Compare December 12, 2024 16:22

ringvold force-pushed the multi-select-tags-mode branch from 4717fbf to 3885edb Compare December 12, 2024 16:25

Add tests for quick_tags and fix failing tags tests

995b45c

ringvold force-pushed the multi-select-tags-mode branch from 3885edb to 995b45c Compare December 12, 2024 16:29

ringvold commented Dec 12, 2024

View reviewed changes

lib/support/live_select_web/live/showcase_live.ex Outdated Show resolved Hide resolved

maxmarcon added 3 commits December 26, 2024 12:21

update docs

d877feb

update docs

4d66dce

update docs again (test)

08ec29f

also cover navigating options upwards in quick_tags mode

d800757

maxmarcon reviewed Dec 26, 2024

View reviewed changes

document selected field

792ee34

maxmarcon requested changes Dec 26, 2024

View reviewed changes

ringvold force-pushed the multi-select-tags-mode branch from 42fc146 to fb80e77 Compare December 27, 2024 11:54

Changes based on review comments

f9607d0

ringvold force-pushed the multi-select-tags-mode branch from fb80e77 to f9607d0 Compare December 27, 2024 11:59

maxmarcon approved these changes Dec 27, 2024

View reviewed changes

maxmarcon self-requested a review December 27, 2024 13:01

fix typo

d1feae9

maxmarcon merged commit a4f6130 into maxmarcon:main Dec 27, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New variant for tags-mode #78

New variant for tags-mode #78

ringvold commented Aug 16, 2024 •

edited

Loading

maxmarcon commented Aug 17, 2024

maxmarcon commented Aug 29, 2024 •

edited

Loading

ringvold commented Sep 3, 2024 •

edited

Loading

maxmarcon commented Sep 10, 2024 •

edited

Loading

ringvold commented Sep 13, 2024

maxmarcon commented Sep 14, 2024

egeersoz commented Sep 19, 2024

maxmarcon commented Sep 19, 2024

ringvold commented Sep 20, 2024 •

edited

Loading

ringvold commented Oct 3, 2024 •

edited

Loading

ringvold commented Dec 12, 2024

ringvold commented Dec 12, 2024

egeersoz commented Dec 18, 2024

maxmarcon commented Dec 26, 2024

maxmarcon left a comment

maxmarcon Dec 26, 2024

maxmarcon Dec 26, 2024

maxmarcon Dec 26, 2024

ringvold Dec 27, 2024

maxmarcon left a comment

maxmarcon Dec 26, 2024

maxmarcon Dec 26, 2024

maxmarcon Dec 26, 2024

maxmarcon Dec 26, 2024 •

edited

Loading

ringvold Dec 27, 2024

maxmarcon Dec 27, 2024

ringvold commented Dec 27, 2024

maxmarcon commented Dec 27, 2024

New variant for tags-mode #78

New variant for tags-mode #78

Conversation

ringvold commented Aug 16, 2024 • edited Loading

Potential performance issue

Breaking changes

maxmarcon commented Aug 17, 2024

maxmarcon commented Aug 29, 2024 • edited Loading

ringvold commented Sep 3, 2024 • edited Loading

Other things of note

maxmarcon commented Sep 10, 2024 • edited Loading

ringvold commented Sep 13, 2024

maxmarcon commented Sep 14, 2024

egeersoz commented Sep 19, 2024

maxmarcon commented Sep 19, 2024

ringvold commented Sep 20, 2024 • edited Loading

ringvold commented Oct 3, 2024 • edited Loading

ringvold commented Dec 12, 2024

ringvold commented Dec 12, 2024

egeersoz commented Dec 18, 2024

maxmarcon commented Dec 26, 2024

maxmarcon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxmarcon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxmarcon Dec 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ringvold commented Dec 27, 2024

maxmarcon commented Dec 27, 2024

ringvold commented Aug 16, 2024 •

edited

Loading

maxmarcon commented Aug 29, 2024 •

edited

Loading

ringvold commented Sep 3, 2024 •

edited

Loading

maxmarcon commented Sep 10, 2024 •

edited

Loading

ringvold commented Sep 20, 2024 •

edited

Loading

ringvold commented Oct 3, 2024 •

edited

Loading

maxmarcon Dec 26, 2024 •

edited

Loading