Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE ember-contextual-components] Contextual components per RFC #64 #12285

Merged
merged 1 commit into from
Sep 21, 2015

Conversation

Serabe
Copy link
Member

@Serabe Serabe commented Sep 3, 2015

emberjs/rfcs#64 (rendered html)

Introduce new syntax and semantics for improving component composition.

TODO

  • Extract current component helper implementation to element_component.
  • Add ember-contextual-components feature flag.
  • Implement hash helper.
  • Implement closure_component.
  • Make component helper handle a component closure.
  • Refactor positionalParams to be expression specific (after [BUGFIX beta] Change preference in positional parameters #12350 this is no longer needed)
  • Implement {{my.component.closure}} syntax. This needs changes in HTMLBars.
  • Clean up the code a bit
  • API docs for (component) (left for a different PR after this has been merged)
  • Squash everything into one commit with the proper tag.

@rwjblue
Copy link
Member

rwjblue commented Sep 6, 2015

Lookin good so far, keep it up!

@Serabe
Copy link
Member Author

Serabe commented Sep 10, 2015

After a great talk with @mixonic, it seems the best road is to do a component cell. Since my streams and cells knowledge is pretty limited, I tried to follow the mut example. In this first draft of closure-component I try to do so but only as far as I think I understand what is happening in there.

I create a ComponentStream that looks quite similar to MutStream:

  1. In the init, I keep the componentPath, params and hash referenced, as any of them can be streams or contain one. I set a proper label and add dependencies as well.
  2. The cell method does what I think the mut version does: reading (as in streams/utils read) the stream content.
  3. I did not add an [INVOKE] method because I am not sure of what it does.

I did not plumbing yet because I am only going as far as I feel I understand properly.

@Serabe Serabe force-pushed the contextual-components branch 2 times, most recently from 30ca578 to a0d0823 Compare September 10, 2015 00:27
just yield a hash:

```handlebars
{{yield (hash
Copy link
Member

Choose a reason for hiding this comment

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

We can give a more generic example here.

Copy link
Member

Choose a reason for hiding this comment

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

donezo

@Serabe
Copy link
Member Author

Serabe commented Sep 18, 2015

What is this error about?

In this other error, I see a lot of them but then the numbers say everything is fine.

@mixonic
Copy link
Member

mixonic commented Sep 18, 2015

@Serabe the first error is sauce labs. Either there is something here incompatible with IE9, or (more likely) sauce labs had an intermittent failure. I'll kick the server.

The "other" error, is the same link in your comment.

@Serabe Serabe force-pushed the contextual-components branch 2 times, most recently from 25e42e4 to 04ea479 Compare September 18, 2015 22:01
This feature moves previous component keyword implementation to
`element-component` and adds a `closure-component` that creates a cell
that closes over the component's path, positional parameters and hash
parameters.

These cells can be nested, merging the previous parameters with the
previous ones.
@mixonic
Copy link
Member

mixonic commented Sep 20, 2015

@wycats @mmun @rwjblue I'd like to merge this PR with an initial implementation of contextual components. It does not include:

  • Explicit GlimmerComponent support/tests, though I expect it is free.
  • Support for {{dot.path}} being considered as a component. That will need HTMLBars effort.
  • Packaging of dependencies. By this I mean the stream for the component cell should not invalidate when a hash/param value changes. Instead that should should be propagated through directly to the invocations. The invocation should be dependent upon the streams of the hash, we should not need to be dirtying the component cell stream. This is an optimization though, and we can fix it while people are experimenting with the core feature behind a flag.

Getting this behind a FF will allow devs to explore the feature and get us early feedback. I hope to tie up the loose ends here as glimmer component lands and ship with 2.3 (ideally).

@Serabe has also edited the component helper documentation to explain nested usage in a branch, so there are docs in the works already.

@mixonic mixonic changed the title [WIP] [FEATURE] Contextual components per RFC #64 [FEATURE] Contextual components per RFC #64 Sep 20, 2015
@mixonic mixonic changed the title [FEATURE] Contextual components per RFC #64 [FEATURE ember-contextual-components] Contextual components per RFC #64 Sep 20, 2015
@mmun
Copy link
Member

mmun commented Sep 20, 2015

If it's feature flagged and works reasonably well I am 👍 merging.

rwjblue added a commit that referenced this pull request Sep 21, 2015
[FEATURE ember-contextual-components] Contextual components per RFC #64
@rwjblue rwjblue merged commit b8e491e into emberjs:master Sep 21, 2015
@rwjblue
Copy link
Member

rwjblue commented Sep 21, 2015

@Serabe - Thank you for working so hard on this!

@Serabe
Copy link
Member Author

Serabe commented Sep 21, 2015

Thanks to you, guys, for the support.

@balinterdi
Copy link
Contributor

@Serabe Fantastic, you pulled it off! Great work!

@rwjblue
Copy link
Member

rwjblue commented Sep 21, 2015

Submitted #12378 with some minor tweaks to the feature flagging syntax.

@DylanLukes
Copy link

This works great, thanks! Forms with the much-lauded simplified syntax are pretty easy to achieve.

Utter barebones:

{{!-- my-form --}}
<form {{action "submit" on="submit"}}>
  {{yield (hash input=(component 'my-form-field' model=model))}}
</form>
{{!-- my-form-field --}}
<div>
  {{#if hasBlock}}
    {{yield}}  {{!-- escape hatch for custom form fields, they happen --}}
  {{else}}
  <label for="...">{{label}}</label>
    {{input value=(mut (get model for))}}
  {{/if}}
</div>
{{!-- usage --}}

{{#my-form model=someModel as |form|}}
  {{form.input label="First Name" for="firstName"}}
  {{form.input label="Last Name" for="lastName"}}
  {{#form.input}}
    {{!-- random stuff here --}}
  {{/form.input}}
{{/model-form}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants