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

Restructure readme to help new users get started #106

Merged
merged 1 commit into from
Mar 23, 2018

Conversation

pauln
Copy link
Contributor

@pauln pauln commented Mar 22, 2018

Combines the "Installation" and "Usage" sections (now after the demo / example etc) to help new users get started with minimal frustration. Fixes #105.

Background

I recently discovered this wonderful addon, but thought that it wasn't working in my Ember app for some reason... until I scrolled past the examples in the readme and spotted the "Usage" section - which told me how to finish setting it up.

Alternatives

I've put the newly-combined "Installation & Usage" section after the examples, demo and "Why use it?" sections - but it could just as well come before them (as the "Installation" section did before).

Combines the "Installation" and "Usage" sections (now after the demo / example etc) to help new users get started with minimal frustration.
@RobbieTheWagner
Copy link
Contributor

Hi @pauln! I'm confused why we want to change this? Just looks like we moved talking about options to a lot further down.

@pauln
Copy link
Contributor Author

pauln commented Mar 23, 2018

Hi @rwwagner90! The key thing is merging the "Installation" and "Usage" sections, so that there isn't a whole bunch of other stuff between them. At the moment, it's very easy to do what I did:

  1. Scroll as far as the examples, stop to watch them
  2. Think "this looks like exactly what I'm after! How do I make it work?"
  3. Scroll back up to the "Installation" section, with its "Options" subsection about telling the addon which element to handle
  4. ember install ember-router-scroll and add the option to point it at the relevant element
  5. Scratch head wondering why it's not working, as there are no errors showing in the console
  6. Hit the "demo" link under "A working example", only to find that it doesn't work there either
  7. Scratch head some more, as there don't appear to be any errors in the console on the demo either
  8. Check the Github issues; find Demo site and dummy app #94, which points out that that demo doesn't work (but the one in the repo description does)
  9. Go back to scratching head about it not working in my own app
  10. Eventually scroll further down the readme and find the "Usage" section, and kick oneself when realising that additional steps are required to get it to work
  11. Follow the additional steps in the "Usage" section
  12. Rejoice at the magic of ember-router-scroll now that it's actually working

I went with "examples first, instructions after" as it's helpful to know that this addon will do what you want before diving in and setting it up - but if you think it'd be better the other way around (per the "alternatives" section in this PR), I'm more than happy to update this PR accordingly.

P.S. Per #94, the demo link in the readme should probably be updated to point at the working version - and/or the working version pushed to this repo's Github Pages, so that the existing link leads to a working version. I didn't do anything about it in this PR, but I can add another commit which updates the link (if that's the preferred option) if you'd like, so that it can all be dealt with by merging one PR - just let me know.

Copy link
Contributor

@RobbieTheWagner RobbieTheWagner left a comment

Choose a reason for hiding this comment

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

Ah, I see what you mean now. Thanks for the PR!

@RobbieTheWagner RobbieTheWagner merged commit f220cd9 into DockYard:master Mar 23, 2018
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.

2 participants