Add view function #109
Merged
Add view function #109
Conversation
This file contains 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
limemloh
approved these changes
Aug 11, 2019
deklanw
reviewed
Aug 13, 2019
examples/todo/src/TodoFooter.ts
Outdated
| selectAll: o.click.mapTo("all") | ||
| })), | ||
| filterItem("Active", selectedClass).output((o) => ({ | ||
| selectActive: o.click.mapTo("all") |
|
Seems good to me. Although, it's another thing to think about. Made an issue for related thoughts I've been having #110 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR is an attempt at addressing a problem pointed out by @deklanw in #107.
The issue is that the TodoMVC example contains a
filterItemfunction for creating a button in a list. Since this list item should tell its parent when it's being clicked it callsoutputon theaelement in it. However, to prevent the outputs from all the constructedfilterItems from colliding the function takes a string and uses that string to change the property name of the outputted stream.This causes two problems:
todoFooterViewit is not apparent thatfilterItemprovides any output sinceoutputis not called on its result. In a sense the output selected intodoFooterView"bubbles" two function levels up. This makes it hard to figure out where the output is actually coming from.This PR fixes the problem by adding a
viewfunction. The idea is thatviewis likemodelViewwithout a model. Functions likefilterItemshould be considered custom components, but, since they don't have a model they should useviewinstead ofmodelView.The only thing
viewdoes is that it turns aComponent<O, any>into aComponent<{}, O>. In other words, it unselects output by turning selected output into available output. Thus, by wrapping a component inviewthe selected output in that component doesn't "bubble" further up unless the parent makes it selected again by callingoutput.I've adapted the TodoMVC example to use the
viewfunction.I also propose that we consider it bad practice for a function to return a component which has already selected output. A function should always return a component with no selected output (just like
modelViewdoes) such that it becomes the callers job to select output and to prevent that output "bubbles" several layers up which makes it hard to track where things come from.I would love to hear your opinion on this @deklanw.