Moving Components Page into its Own Category #1482
Conversation
|
This is awesome, thanks Sarah! I'll give it a deeper look next week. For anyone else who wants to help review this, the preview deployment can be found at https://deploy-preview-1482--vuejs.netlify.com/v2/guide/components.html |
|
@yyx990803 Just a note here: I'm working with Sarah to polish this a bit more and combine it with some of my own local work, so you can put off reviewing until later this week - I'll re-tag you. 🙂 |
|
@chrisvfritz cool, thanks! |
…inks to more in-depth pages
|
Update: I just finished the rough drafts of all these pages and doing my second pass today. If all goes well, this might be ready for review tonight! Once approved, we'll just have to set up redirects for all the broken links. Not looking forward to that part. 😅 |
|
OK, I still need to do one more polish pass for a few of these pages, but it should be worthwhile to review now. 🙂 |
chrisvfritz
requested review from
yyx990803,
LinusBorg and
phanan
and removed request for
chrisvfritz
|
@sdras I can't tag you for review since it's your PR, but I'd also love to see your comments, since I'm the last one to touch each page. 😄 |
|
Sounds good, I should have some time next week to go through it :) |
|
Yay! I think it'd be really nice if we could have this in, in time for VueConf. 🙂 |
|
I can't approve my own PR but I just looked through all the changes, and it looks good to me 👍 |
|
|
||
| ## Event Names | ||
|
|
||
| Unlike components and props, event names don't have provide any automatic case transformation. Instead, the name of an emitted event must exactly match the name used to listen to that event. For example, if emitting a camelCased event name: |
There was a problem hiding this comment.
event names don't
haveprovide any automatic case
have should be dropped here.
| <my-component v-on:my-event="doSomething"></my-component> | ||
| ``` | ||
|
|
||
| The reason for this is that unlike components and props, event names will never be used as variable or property names in JavaScript, so there's no reason to use camelCase or PascalCase. Additionally, `v-on` event listeners inside DOM templates will be automatically transformed to lowercase (due to HTML's case-insensitivity), so `v-on:myEvent` would become `v-on:myevent` -- making `myEvent` impossible to listen to. |
There was a problem hiding this comment.
The reason for this is that uUnlike components and props, event names will never be used as variable or property names in JavaScript, so there's no reason to use camelCase or PascalCase.
|
|
||
| > New in 2.2.0+ | ||
|
|
||
| By default, `v-model` on a component uses `value` as the prop and `input` as the event, but some input types such as checkboxes and radio buttons may want to use the `value` attribute for a [different purpose](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/checkbox#Value). Using the `model` option can avoid a conflict in such cases: |
There was a problem hiding this comment.
can avoid
aconflict in such cases
The indefinite article "a" is unnecessary.
There was a problem hiding this comment.
I'm going to leave this in actually, because while many dialects of English would allow omitting the "a", I think it would sound strange in American English.
There was a problem hiding this comment.
About to say the same. How is "a" unnecessary?
There was a problem hiding this comment.
@znck can correct me if I'm wrong, but I think in Indian English it's more common, and even in American English there will be no article in some contexts. For example, "there was conflict between two countries."
There was a problem hiding this comment.
Yeah, in American English, omitting the a in this case would sound strange.
There was a problem hiding this comment.
I often (read: almost always) rely on Grammarly for this, and it seems to dislike omitting the article in @chrisvfritz's example:
There was a problem hiding this comment.
@phanan 👍 for Grammarly - great tool! I do disagree with it sometimes though. 😄 I think this is a phrase people use on the news quite often.
| </label> | ||
| ``` | ||
|
|
||
| In that case, the `.native` listener in the parent would silently break. There would be no errors, but the `onFocus` handler wouldn't be called when we expected it to. |
There was a problem hiding this comment.
There would be no errors, but the
onFocushandler wouldn't be called when we expected it to.
Can we simplify this statement?
There was a problem hiding this comment.
Hm, nothing is coming to mind, but I'm definitely open to suggestions!
|
|
||
| > New in 2.3.0+ | ||
|
|
||
| In some cases we may need "two-way binding" for a prop. Unfortunately, true two-way binding can create maintenance issues, because child components can mutate the parent without the source of that mutation being obvious in both the parent and the child. |
| <text-document v-bind.sync="doc"></text-document> | ||
| ``` | ||
|
|
||
| This has the effect of adding `v-on` update listeners for not only `title`, but also any other properties on the `doc` object. |
There was a problem hiding this comment.
Instead of This, we can repeat the context.
The
.syncmodifier when used withv-bindhas the effect of ...
There was a problem hiding this comment.
Hm, this seems redundant to me since we just established the context in the previous sentence. @sdras What do you think?
There was a problem hiding this comment.
Does code snippets between paragraphs break context?
For large code snippets, it happens. I tend to go back to last line before continuing to next paragraph. For one line snippet, I am not sure. Moreover, the amount code breaking reading context may vary from person to person.
There was a problem hiding this comment.
I prefer "This" here. One line of code in between is not enough to repeat the statement.
There was a problem hiding this comment.
@znck Yeah, I agree with the principle in general, but in this particular case I think it will be better for most people to leave it out.
There was a problem hiding this comment.
Yeah, I read it through and it seemed redundant to change it, it's pretty clear to me what this refers to. But that's a good thing to look out for!
|
|
||
| You'll notice that if you select a post, switch to the _Archive_ tab, then switch back to _Posts_, it's no longer showing the post you selected. That's because each time you switch to a new tab, Vue creates a new instance of the `currentTabComponent`. | ||
|
|
||
| This is normally useful behavior, but in this case, we'd really like those tab component instances to be cached once they're created for the first time. To solve this problem, we can wrap our dynamic component with a `<keep-alive>` element: |
There was a problem hiding this comment.
It's better to restate the context, instead of using this.
Not caching component instances is normally useful behaviour, but in above example, we'd like the tab component instances to be cached ...
There was a problem hiding this comment.
I agree with you in this case. 🙂 Updated.
|
|
||
| ## Async Components | ||
|
|
||
| In large applications, we may need to divide the app into smaller chunks and only load a component from the server when it's actually needed. To make that easier, Vue allows you to define your component as a factory function that asynchronously resolves your component definition. Vue will only trigger the factory function when the component actually needs to be rendered and will cache the result for future re-renders. For example: |
There was a problem hiding this comment.
We can remove actually word from this paragraph as the meaning of the statement remains same.
...only load a component from the server when it's
actuallyneeded.
...when the componentactuallyneeds to be rendered and will cache the result...
|
It looks like it might not be possible to alias hash links (e.g. |
src/v2/guide/components.md
Outdated
|
|
||
| Every component instance has its own **isolated scope**. This means you cannot (and should not) directly reference parent data in a child component's template. Data can be passed down to child components using **props**. | ||
| If you try this in your template however, Vue will show an error, explaining that **every component must have a single root element**. Vue enforces this limitation to enable some interesting features, which you'll learn about later. |
There was a problem hiding this comment.
This is an actual technical limitation during the implementation of Vue 2, and we may eventually support fragments as component template root in the future, so I'd avoid phrasing it as if we did it to enable specific features.
src/v2/guide/components.md
Outdated
| <button-counter v-on:increment="incrementTotal"></button-counter> | ||
| <button-counter v-on:increment="incrementTotal"></button-counter> | ||
| </div> | ||
| <ul> |
There was a problem hiding this comment.
This example isn't entirely correct - per the spec <ul> does have restriction on what elements can go inside, but most browser don't actually enforce it with the hoisting behavior. <table> and <select>, on the other hand, do enforce it with hoisting.
| }, | ||
| // ... | ||
| } | ||
| ``` |
There was a problem hiding this comment.
I think we should move the tip explaining the ES2015+ shorthand up here, since this is the first time the usage has appeared. In addition, we should also explain the naming resolution rules - i.e. the fact that you can use it in the template both as component-a and ComponentA.
| </my-component> | ||
| ``` | ||
|
|
||
| <p class="tip">However, <code>inline-template</code> makes the scope of your templates harder to reason about. As a best practice, prefer defining templates inside the component using the <code>template<code> option or in a <code><template><code> element in a <code>.vue<code> file.</p> |
There was a problem hiding this comment.
Missing slash in ending </code> tags here
|
|
||
| ## Controlling Updates | ||
|
|
||
| Thanks to Vue's Reactivity system, always knows when it needs to update (if you use it correctly). There are edge cases, however, when you might want to force an update, despite the fact that no reactive data has changed. Then there are other cases when you might want to prevent unnecessary updates. |
There was a problem hiding this comment.
"Thanks to Vue's Reactivity system, [it] always knows when it needs to update"
There was a problem hiding this comment.
If this is fixed, it's not showing up for me, just FYI (maybe not pushed yet?) I thought I would call it out just in case
|
|
||
| ### Forcing an Update | ||
|
|
||
| <p class="tip">If you find yourself needing to force in update in Vue, in 99.99% of cases, you've made a mistake somewhere.</p> |
|
🎉 |
ref #1230
I've heard from a few people now that this page is too long and therefore a little confusing, so I split it up. I actually didn't the issue or Evan's suggestion for the sidebar in the issue until now, but the sections I chose are actually pretty close. Happy to change things around, though. Just aiming for clarity and ease of use, suggestions welcome.
Feel free to tag more reviewers as well.
REVIEW NOTES FROM CHRIS
As you review, these are things I'd like to address now:
For the sake of getting this merged in a somewhat timely manner, I'd prefer reviews did not include ideas for further improvements. Please create new issues for those wonderful ideas, as we can keep improving this after it's merged. 🙂