svelte-typeahead
svelte-typeahead copied to clipboard
add `selection`
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)
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
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.
Sorry, to less sleep last night, I missed updating the afterUpdate()
function.
Is fixed now.
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
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.
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() {}
.
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
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
.
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;
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.