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

Clarify "why Resources", add more examples, and what *not* using resources is like. #361

Merged
merged 10 commits into from
Feb 3, 2022

Conversation

NullVoxPopuli
Copy link
Owner

@NullVoxPopuli NullVoxPopuli commented Jan 30, 2022

@NullVoxPopuli NullVoxPopuli force-pushed the readme-updates branch 3 times, most recently from 5a99a1b to d123c76 Compare January 31, 2022 04:27
README.md Outdated Show resolved Hide resolved
{{this.records}}
```

In this example, `trackedFunction` will make a call to [StarWars API](https://swapi.dev/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it will be better than putting it into words, paste a code there? Or maybe both?

@action
switchToPlanets() {
  this.endpoint = 'planets';
}

README.md Outdated Show resolved Hide resolved
>
> Ember templates are declarative. When we design a component, like the profile component from our previous example, we are specifying declaratively the HTML that should be rendered. If the data used in the templates ever updates, then Ember will update the rendered output as well, and we don't have to worry about the details. We don't have to tell Ember which specific steps to take, and when - it figures everything out for us.

<div style="width: 100%; text-align: right;">
Copy link
Contributor

Choose a reason for hiding this comment

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

It is tiny bit weird how this is rendered without margins around. The next paragraph then sticks to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would this format be better for citing?

README.md Outdated
For example, if you wanted to lazily and reactively load data based on
an `@argument` passed to a component

<details><summary>In vanilla Ember, no addons</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the collapsibles, but it's just hard to notice when scanning the rendered document. I wonder if having sub-sub-section header right above each of them would help? Just a thought, feel free to ignore.

Copy link
Owner Author

Choose a reason for hiding this comment

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

imo, there is too much in this README to not have collapsibles for secondary information. Imagine in the following examples there were also 200+ lines of code

Copy link
Contributor

Choose a reason for hiding this comment

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

I love the collapsibles. I just wanted pointed out that while scanning the document, they are easy to miss. Ultimately an issue with how browsers / github decided to style those.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah. I'm gonna try to figure out how to make this it's own page, so the examples aren't needed

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@sukima
Copy link
Contributor

sukima commented Jan 31, 2022

NOTE: if you are also using ember-could-get-used-to-this, @use is not compatible with this library's LifecycleResource, and useResource does not work with ember-could-get-used-to-this' Resource. However, both libraries can still be used in the same project.

This line assumes a lot of detailed knowledge about the topic. Maybe it is a good idea to add a section later in the README that can hand hold the reader through this decision making caveat and link to it where the note is?

@NullVoxPopuli
Copy link
Owner Author

assumes a lot of detailed knowledge about the topic

Yeah 🤔

I think maybe it makes sense to just remove that section?
Esp with: https://github.com/NullVoxPopuli/ember-resources/pull/107/files

README.md Outdated Show resolved Hide resolved
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Feb 1, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7dbdae8
Status: ✅  Deploy successful!
Preview URL: https://5cd26c96.ember-resources.pages.dev

View logs

@NullVoxPopuli NullVoxPopuli merged commit e31dff0 into main Feb 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2022

🎉 This PR is included in version 4.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

5 participants