Small improvements in the quiescent todomvc example#2
Open
janherich wants to merge 5 commits intolevand:gh-pagesfrom
Open
Small improvements in the quiescent todomvc example#2janherich wants to merge 5 commits intolevand:gh-pagesfrom
janherich wants to merge 5 commits intolevand:gh-pagesfrom
Conversation
I believe the correct TodoMVC behavior is to hide footer if there are no items present in todo-list.
This commits implements todo items hiding according to filter state in a more (i believe) idiomatic way. Instead of relying on css class to show/hide particular DOM element, when we specify parent component (TodoList) we first filter through app items and call child component (Item) only when it's not hidden according to our display logic (determined by hidden? function). It has a nice side-effect that we don't have to pass [item filter] tuple to Item component as just item us sufficient and also fixes the bug that whenever our filter state was set to 'complete' and we added new todo item, this item was shown in the list even when it should really be hidden in my opinion.
I believe that put! calls are more idiomatic way to put value on core.async channel from callback as we are not interested in establishing state machine created by go macro, it's basically -> 'put the value on channel and i am not interested in rendezvous wit the consumer, there is no need for coordination, thanks' I hope my understanding of core.async is correct though, so please review carefully :)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Luke,
With this pull request, i did some improvements to your todomvc quiescent sample, i divided it into 3 commits, each one implementing one change.
There is still some work to do, for example 'toggle all' functionality is not working at the moment, i will try to fix that too.