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

[DOCS octane] Break out the upgrade guide into individual pages #1096

Merged
merged 1 commit into from Oct 22, 2019

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Sep 23, 2019

Breaks out the upgrade guide into individual pages, including one that hasn't yet been written on Modifiers (I'll be focusing on that next).

Since we can't have nested sections yet, and we don't want to make the Octane upgrade guide a top level section, I opted to put a dash-bullet character at the beginning of the page titles. This is much better for reading the navigation in my opinion:

Screen Shot 2019-09-23 at 1 05 19 PM

But it does result in a bit of an awkward top level title on the page itself:

Screen Shot 2019-09-23 at 12 57 15 PM

I'm also unsure what the a11y impact of this would be. Personally, I would take the slightly awkwardness of the page title for increased clarity in navigation for now, and focus on getting the ability to have nested navigation asap (it would also be good for the new tutorial), but am open to suggestions. cc @mansona, I would love to help add the nesting to guidemaker if you have any guidance on that

@MelSumner
Copy link
Member

MelSumner commented Sep 23, 2019

@pzuraq do you think this change proposal should go to https://github.com/ember-learn/guidemaker-ember-template? Also maybe a different proposal, one that allows three levels in the sidebar.

@jenweber
Copy link
Contributor

Ideally we will not use the "octane" keyword in these. Links or their redirects live forever, so it would be good to keep these pages reusable.

@pzuraq
Copy link
Contributor Author

pzuraq commented Sep 23, 2019

That’s actually why I put the octane keyword in each of them, so they wouldn’t accidentally overlap with future guides for future editions. These are guides targeted for a specific edition upgrade, so it makes some sense to namespace them.

Happy to remove the keyword, but I think that could be an issue when we release the next edition after Octane and need a new cheat sheet, etc.

@jenweber
Copy link
Contributor

jenweber commented Oct 2, 2019

We should wait to merge this until after #1104 and the file renaming.

@pzuraq pzuraq force-pushed the docs/octane/break-out-octane-upgrade-guide branch from 61675e2 to e566c44 Compare October 16, 2019 18:58
@pzuraq
Copy link
Contributor Author

pzuraq commented Oct 16, 2019

This has been updated with nesting! Ready for final review 😄

@pzuraq pzuraq force-pushed the docs/octane/break-out-octane-upgrade-guide branch from e566c44 to 92d8ae2 Compare October 16, 2019 20:43
@jenweber
Copy link
Contributor

Looking good!

Screen Shot 2019-10-21 at 3 23 01 PM

@jenweber
Copy link
Contributor

I also tested the tabbing behavior and it seems good.

@pzuraq pzuraq force-pushed the docs/octane/break-out-octane-upgrade-guide branch from 92d8ae2 to fdb9b71 Compare October 21, 2019 19:32
Copy link
Contributor

@jenweber jenweber left a comment

Choose a reason for hiding this comment

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

Thank you, Chris! This is so much easier to read and I appreciate all the work you did around the edges to make this happen.

@jenweber jenweber removed the on hold label Oct 22, 2019
@jenweber jenweber merged commit 554d0a3 into octane Oct 22, 2019
@jenweber jenweber deleted the docs/octane/break-out-octane-upgrade-guide branch October 22, 2019 12:37
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.

None yet

3 participants