Skip to content
This repository was archived by the owner on May 26, 2019. It is now read-only.

[New Guide] Composing Components#66

Merged
mixonic merged 1 commit intoemberjs:masterfrom
knownasilya:patch-1
Jul 24, 2015
Merged

[New Guide] Composing Components#66
mixonic merged 1 commit intoemberjs:masterfrom
knownasilya:patch-1

Conversation

@knownasilya
Copy link
Contributor

So one of the biggest issues with components, is that users don't know how do use them, they are new and scary. This guide helps users dive into using components as they were meant to be used (beyond the single lonesome component).

This guide takes the Data Down and Actions Up approach that is being favored and will be prevalent once all of the Ember 2.0 component features are in (mut, attrs, etc).

preview rendered

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to bind in some data otherwise it's unclear where those titles are coming from below...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep em coming! Would love to make this a super nice guide!

@mgenev
Copy link

mgenev commented Mar 23, 2015

I'll review and make suggestions. Thanks for the link to this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to pass users in here:

{{#user-list users=model sortBy='name' as |user|}}

@knownasilya
Copy link
Contributor Author

I think this is ready cc @rwjblue @trek

I can squash commits if you agree.

@knownasilya knownasilya changed the title Composing Components [New Guide] Composing Components Mar 27, 2015
@knownasilya
Copy link
Contributor Author

Ping @trek

@bf4
Copy link

bf4 commented Jun 30, 2015

Seems like momentum on this PR has been lost...

@knownasilya
Copy link
Contributor Author

@bf4 I think it's due to the fact that actions have changed in 1.13, so maybe a revision is in order.

@knownasilya
Copy link
Contributor Author

I'll try to tackle the changes this weekend. Would love for some people to be ready to check it once I have those in.

@locks
Copy link
Contributor

locks commented Jul 12, 2015

@knownasilya ping me if/when you do

@knownasilya
Copy link
Contributor Author

Already started, please review up until the CONTINUE HERE section.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one-off form?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't know how futureproof default form of a component is, given CLI's default template for a component is {{yield}}

@knownasilya
Copy link
Contributor Author

Thanks for the comments! Will sort them out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it sending data down though, if the yielded template is in the same context as the yielding component?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Down to the block/child components/elements.

@knownasilya
Copy link
Contributor Author

Sounds like it could be done, as long as we know what the TOC is for that section.

@michaelrkn
Copy link
Contributor

Yeah, that's a bit tough, because I imagine the Components section will have to be re-thought a bit when routable components lands. I'd lean towards just incorporating your ideas into the guides as they are now and not worry too much about the future, because the future is uncertain and your ideas are good :) But maybe I am not enough of a visionary. @trek, do you have any guidance here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That this first example uses #each and yield together is a bit intense.

At the least I suggest where we output a headline and yielded body for each post.

@knownasilya
Copy link
Contributor Author

@mixonic thanks for the comments.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subexpression, targetObject I don't think they will knoe what that means.

(action reads a property off the actions hash of the current context, then creates a closure over that function and any arguments you pass.

@mixonic
Copy link
Member

mixonic commented Jul 23, 2015

@knownasilya sorry this took me so long to look at. I am in theory 👍 on a merge if we get it polished up. @trek gets veto power if he wants to wait on an alternative TOC, but the worst that happens is that is gets refactored into that new TOC when it arrives IMO.

@knownasilya
Copy link
Contributor Author

Will have a look at the comments and make the changes as soon as I can, hopefully tonight.

@knownasilya
Copy link
Contributor Author

@mixonic @michaelrkn all set

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we saw in the previous section reads awkwardly.

mixonic added a commit that referenced this pull request Jul 24, 2015
[New Guide] Composing Components
@mixonic mixonic merged commit b02823a into emberjs:master Jul 24, 2015
@mixonic
Copy link
Member

mixonic commented Jul 24, 2015

👍 good improvements! Thanks for pushing on this.

@locks
Copy link
Contributor

locks commented Jul 24, 2015

🍪 for you @knownasilya :) thanks

@knownasilya knownasilya deleted the patch-1 branch July 24, 2015 14:04
@knownasilya
Copy link
Contributor Author

Thanks all for proof reading, couldn't have done it without you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants