svelte-typeahead icon indicating copy to clipboard operation
svelte-typeahead copied to clipboard

add `selection`

Open Amerlander opened this issue 3 years ago • 10 comments

Introducing a filter for selecting items additional to filter and disable. This gives selected items an class, which could be styled from outside.

Actually I keept selected as class name, which was used for active/focused items. So this might be a breaking change and I would totally undertand if you dislike this. For the former selected class I introduced active and changed the styling to a light blue border.

Here is the code where I use all functions, if you think this change is reasonable I will also write an update for the readme:

<script>
    import Typeahead from "svelte-typeahead";
    import { mdFieldsAll } from './stores.js';

    function handleSelect(e) {   
        let i = e.detail.originalIndex;
        $mdFieldsAll[i].selected = !$mdFieldsAll[i].selected;
    }

    const extract = (item) => item.name_human_readable + ' ['+ item.title +']';

    const disable = (item) => item.required;

    const selection = (item) => item.selected;

    const filter = (item) => item.hidden;

  </script>

<div data-svelte-mycomponent>
{#await $mdFieldsAll}
  <p>...waiting</p>
{:then data}
    <Typeahead {data} {extract} {selection} {disable} {filter} inputAfterSelect='keep' focusAfterSelect='true' let:result on:select="{handleSelect}">
        <div>
            <h3>{@html result.string}</h3>
            <p>{@html result.original.explica}</p>
        </div>
    </Typeahead>
{:catch error}
  <p style="color: red">{error.message}</p>
{/await}
</div>

<style>
// overwrite default styling of selected items
    [data-svelte-mycomponent] :global([data-svelte-typeahead] .selected) {
        background: rgb(200,260,255);
    }
</style>

I further changed the global css classes a bit, to keep their scope in the component and its sub-components: :global([data-svelte-search] label) to [data-svelte-typeahead] :global([data-svelte-search] label)

Amerlander avatar Feb 21 '21 15:02 Amerlander

I just also pushed a change which prevents disabled items from beeing select by keyboard or as first entry. Please have a look at before merging: https://github.com/metonym/svelte-typeahead/pull/13/commits/3ee7b6e06d57379799b4b7041867658a6d5d9d97

Amerlander avatar Feb 21 '21 19:02 Amerlander

its self-referencing, so I think it has to be a function. And its also called in afterUpdate() to handle if the first element is disabled. So we would have the logic in two places if we keep it in the markup.

Amerlander avatar Feb 21 '21 20:02 Amerlander

Sorry, to less sleep last night, I missed updating the afterUpdate() function. Is fixed now.

Amerlander avatar Feb 21 '21 20:02 Amerlander

A couple of things:

  • As much as possible, I prefer business logic triggered by events to be in the markup – even if it means duplication
  • I'm not sure if incrementing the index for a disabled item would fix the issue; you would need a while loop to find the next non-disabled item

metonym avatar Feb 21 '21 20:02 metonym

I increment the selectedIndex as long as the selectedItem turns out to be disabled. To prevent infinite loop I stop after result.lenght-times. In my test it actually is fixing the issue. I just was mentally not able to bring it to github in the first try.

I had different approaches now, first is the function way of this pr, tested and working

function setNextSelectedIndex(i = 0) {
    i += 1; // index for stop the itteration
    selectedIndex += 1;
    if (selectedIndex === results.length) {
      selectedIndex = 0; // start over at the beginning of the array
    }
    if(results.lenght == 0 || !(selectedIndex in results) || i > results.lenght) {
      selectedIndex = -1; // no result found, gone trough the whole array
    } else if(results[selectedIndex].disabled) {
      setNextSelectedIndex(i); // no result found, not yet gone trough the whole array
    }
  };

In think an while loop would look something like that, but i havent tested yet

    selectedIndex += 1;
    if (selectedIndex === results.length) {
      selectedIndex = 0;
    }
    let i = 0;
    while(selectedIndex in results && results[selectedIndex].disabled && results.lenght > i) {
        i += 1;
        selectedIndex += 1;
        if (selectedIndex === results.length) {
          selectedIndex = 0;
        }
    }
    if(results[selectedIndex].disabled) {
      selectedIndex = -1;
    }

There was one approach with a lot of .filter()-stuff I gave up on.

I found the for working pretty nice and will put this into this pr. Incrementation:

    for (selectedIndex++;(selectedIndex in results && results[selectedIndex].disabled); selectedIndex++) { }
    if(!(selectedIndex in results) || (selectedIndex in results && results[selectedIndex].disabled)) {
        selectedIndex = results.findIndex(result => !result.disabled);
    }

for the decrementation works similar:

    for (selectedIndex--;(selectedIndex in results && results[selectedIndex].disabled); selectedIndex--) { }
    if(!(selectedIndex in results) || (selectedIndex in results && results[selectedIndex].disabled)) {
        let reverseselectedIndex = results.slice().reverse().findIndex(result => !result.disabled) + 1;
        selectedIndex = (reverseselectedIndex == -1) ? -1 : (results.length - reverseselectedIndex);
    }

In the afterUpdate() function we can just find the first selectable item by results.findIndex(result => !result.disabled);, so there even is no repeating code.

Amerlander avatar Feb 21 '21 22:02 Amerlander

it feels a bit weird to have a for-loop without an acutal function.

So I don't know whether to write for(); or for() {}.

Amerlander avatar Feb 21 '21 22:02 Amerlander

A couple of requirements:

  • when all items are disabled, I get stuck in an infinite loop when using the arrow keys
  • findIndex is not supported in IE

metonym avatar Feb 21 '21 23:02 metonym

Surely there's a more "Svelte" way of doing this.

Just thinking a loud here. Say that you have a reactive assignment that only tracks non-disabled items:

$: enabledResults = results.filter(item => !item.disabled);

Then when incrementing/decrementing using arrow keys, you could use enabledResults instead of results to determine to pivot the selectedIndex.

metonym avatar Feb 21 '21 23:02 metonym

Ok, I'll try again. Thanks for your patience, I`m pretty new to svelte.

My first approach still uses .findIndex(). Is IE in Svelte an issue? I`m not that much into it and hope when the support for IE stops in 6 Months IE will not be an issue anywhere else too, but I thought Svelte is in its standard configuration not compatible to IE at all. Regarding this I read this blog article: https://blog.az.sg/posts/svelte-and-ie11/ So do we have to keep IE in mind while developing svelte components, or is this handled by polyfills anyways?

  $: enabledResults = results.filter(item => !item.disabled);
         case 'ArrowDown':
          e.preventDefault();
          enabledIndex = enabledResults.findIndex(item => item == results[selectedIndex]);
          if(enabledIndex == -1) break;
          enabledIndex += 1;
          if (enabledIndex === enabledResults.length) {
            enabledIndex = 0;
          }
          selectedIndex = results.findIndex(item => item == enabledResults[enabledIndex]);
          break;
        case 'ArrowUp':
          e.preventDefault();
          enabledIndex = enabledResults.findIndex(item => item == results[selectedIndex]);
          if(enabledIndex == -1) break;
          enabledIndex -= 1;
          if (enabledIndex < 0) {
            enabledIndex = enabledResults.length - 1;
          }
          selectedIndex = results.findIndex(item => item == enabledResults[enabledIndex]);
          break;

Amerlander avatar Feb 22 '21 00:02 Amerlander

I'll go with findIndex, since IE already need at least one polyfill for promise. I wrote a note in the readme. If you find something what doesn't need a polyfill I'll take this of course.

Amerlander avatar Feb 23 '21 10:02 Amerlander