Create a 'loading results' indicator.

[reficul31]

While the results are being loaded I think we could add an asynchronous call that issues a "loading results" action. It can basically display that results are being loaded or we can even add a progress bar.

[Treora]

I think the required actions are already there (refreshSearch and setSearchResult), but (1) they should toggle a flag in the state to indicate that search is busy, and (2) the overview should read this flag and show a nice loading indicator when appropriate. I can fix the first task if somebody wants to pick up the second (or feel free to do both).

[reficul31]

@Treora

I think the required actions are already there (refreshSearch and setSearchResult), but (1) they should toggle a flag in the state to indicate that search is busy, and (2) the overview should read this flag and show a nice loading indicator when appropriate.

Or we could make a new epic that triggers a new reducer that changes the state to loading. And when the results are ready we change the state back to display.

I would like to take up both the tasks. I think the approach to follow is

1. Start by making animation for loading.
2. Set state and trigger animation.
3. Add an epic that triggers the state to loading.
4. Once the searchResult var is ready change the state to ready.
5. Stop animation and display results

I hope I didn't misinterpret.

[obsidianart]

I would change 1 in "Create a component for loading", it's very easy to drop it with something like

if (search.isFetching) return <Loading />

[reficul31]

@obsidianart Just for clarification do you want loading to be a dumb component? In my opinion I think making something like this would be cleaner(?)

{search.isFetching?<div class='loading></div>:null}

I'm sorry if I misunderstood you completely.

[Treora]

Epics work too, but may make it more complicated. I would say try write it out and see what results in the most elegant and understandable code.

As for starting/stopping an animation; I assume it could just be a simple {searchIsLoading ? <some-loader-icon-component /> : null}. Not sure where, next to the query input perhaps? We could also colour the input's background slightly differently as an indicator. A progress bar is impossible as we do not know the progress.

One thing to note by the way: it can sometimes happen that multiple searches are running in parallel if one took a long time and the query changed in the meantime and started a new search (unfortunately we cannot cancel the running search action). Perhaps the state would have to keep count of how many searches are running, rather than just a boolean, and it would show the loading indicator when the count is more than zero.

[reficul31]

@Treora Duly noted. I will start by making the component then. I shall use epics for now. If the code gets too complicated we can remove for a simple function.

[obsidianart]

@Treora If you can't stop a running search but you allow multiple you will have other problem in the future (merged result, incorrect results, etc) but it's outside fo the scope of this. @reficul31 The loader is a component with an icon as @Treora clarified. Whatever is a return or not depends on the rest of the code. The ternary isn't needed for how JSX works this is the common patter: {search.isFetching && <div class='loading />} or with the component {search.isFetching && <SomeLoader />}

[reficul31]

@obsidianart @Treora

If you can't stop a running search but you allow multiple you will have other problem in the future (merged result, incorrect results, etc) but it's outside fo the scope of this.

So I guess a boolean will work with it what we are trying to implement here?? Also an icon component should be in its own file or I can make it in the same file as overview.jsx? Ps. We can create like a universal icon component with something like this

const icons = {
  trash: 'trash-icon',
};

export default = (props)=>(
<svg width="22" height="22" viewBox="0 0 1024 1024">
    <path d={icons[props.icon]}></path>
  </svg>
);

This allows for future devs to make icon components easier

[Treora]

@obsidianart https://github.com/obsidianart @Treora https://github.com/Treora

If you can't stop a running search but you allow multiple you will
have other problem in the future (merged result, incorrect
results, etc) but it's outside fo the scope of this.

So I guess a boolean will work with it what we are trying to implement here??

For now we do have the possibilities that multiple searches run in parallel, so a counter may be needed.

@obsidianart: I think the only problem is the computational load: when search results are received, the code discards them if they seem to have become irrelevant, so if I implemented that correctly it should not cause incorrect results (see the actions linked above).

[obsidianart]

great. Observable solve this problem in an elegant way but I wouldn't suggest worrying about it now. All good for me, counter it's probably a good idea in this scenario.

[reficul31]

Also an icon component should be in its own file or I can make it in the same file as overview.jsx?

Right now should I make some sort of an animation of loading or just a simple loading now?? Also should it be like beneath the search bar or like a very flamboyant animation in the middle of the page?

Right now I am making an animation that looks something like that. The dots move up and down. Is this appropriate?

[Treora]

As for any contributor, do what you what you judge to be best, then tell us about it. :)

I wonder if the previous results should still be visible while the new ones are loading though.. That's why I thought of putting a small animation besides the query input bar. But as said, if you make something convincingly prettier than what I had in mind, that's totally fine too.

[reficul31]

I have sent a PR on my view of how the issue should be resolved. Currently when the searching query is typed it plays the loading icon and the results in the list below disappear. The loading icon is a simple 3 dot loading icon which go up and down out of sync. I hope my approach towards the problem was correct. Feedback on the design of the loading animation is welcome as I have no problem changing it.

Epics work too, but may make it more complicated. I would say try write it out and see what results in the most elegant and understandable code.

@Treora sorry for using epics. I just found the code to be more compact that way. Please review the pull request if the code seems out of format, I will change it immediately.

[blackforestboi]

Finished with #57

Great stuff, thanks @reficul31!