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

Refactored Installation page to no longer use tabs #11050

Merged

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Oct 2, 2017

Resolves #11043

The installation page previously used a one-off tab mechanism to visual limit the content shown on the screen. Unfortunately this mechanism did not work correctly in production on the new website, causing some links to show the incorrect content.

Ultimately this PR resolves the issue by removing all of the embedded HTML+CSS in the installation.md file in favor of standard markdown syntax. I believe this is the best short-term solution and that we can revisit the UI/UX of this page in the future to improve it (cc @joecritch).

screen shot 2017-10-02 at 3 17 48 pm

The anchor tags should now work correctly, eg https://deploy-preview-11050--reactjs.netlify.com//docs/installation.html#using-a-cdn

For posterity, I also considered the following alternative approaches.


Breaking the sub-sections into their own pages

Pros:

  • Keeps the document uncluttered by limiting the presented info (in a manner similar to the tabs)

Cons:

  • Not obvious what the next/prev links should be on the Installation page or any of the sub-pages
  • Could not redirect from any existing external links to eg /docs/installation.html#using-a-cdn

screen shot 2017-10-02 at 3 08 12 pm


Keeping content in a single page but with side-nav headers

screen shot 2017-10-02 at 3 15 14 pm

Pros:

  • Nice high-level overview of the sections, similar to tab mechanism

Cons:

@reactjs-bot
Copy link

Deploy preview ready!

Built with commit 66b3e90

https://deploy-preview-11050--reactjs.netlify.com

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

OK

@gaearon
Copy link
Collaborator

gaearon commented Oct 2, 2017

Can we maybe add a note to Babel section that once again mentions you don't need to configure anything if you use CRA? For some reason people just skip ahead there and get lost :-(

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 2, 2017

Sounds like you have a somewhat specific idea of what you want to be added where, and I don't. Mind posting a follow-up a PR that does this? I'd be happy to review it.

@bvaughn bvaughn merged commit 75ad1a9 into facebook:master Oct 2, 2017
@bvaughn bvaughn deleted the website-installation-page-active-tab branch October 2, 2017 22:45
@gaearon
Copy link
Collaborator

gaearon commented Oct 2, 2017

I don't really have a solution to that problem. (Well, tabs were a solution but they introduced other problems.)

I just know that people don't read titles on one page, skip ahead, and then don't realise they've missed the section they needed.

I've seen countless reports from people who tried CRA and then for some reason continued reading down and attempted to install webpack and Babel on top of it. They didn't believe CRA just works out of the box. They saw "Enabling JSX and ES6" and thought they need to do it even if it's in a completely separate section.

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 3, 2017

I'd be happy to spend some time going over the docs with a fresh set of eyes, maybe trying to tackle some problems like this, if Sophie thinks it's worth the time spent. Will add it to the discussion notes for today's sync.

@sophiebits
Copy link
Collaborator

Maybe it makes sense to just split these into three separate pages?

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 4, 2017

@sophiebits Like I described in the PR description, "Breaking the sub-sections into their own pages" ? Or something else...

@sophiebits
Copy link
Collaborator

Yes, that. 😳

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 4, 2017

Yeah, i'm not opposed to doing that. I mostly held off because I didn't know how many existing links it might break. (We can clearly search within the site and fix any internal ones, but I've been finding out there are a lot of external links to the site.)

@sophiebits
Copy link
Collaborator

JS redirect?

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 4, 2017

Yeah I guess we could do that but it seems...shitty. We would have to replace the installation.md markdown+template with a new installation.html.js page so we could do the redirect and we'd need to remember to keep that redirect in-sync any time we made structural changes to either of the newly-added markdown files (which seems a tad bit fragile).

@sophiebits
Copy link
Collaborator

we'd need to remember to keep that redirect in-sync any time we made structural changes to either of the newly-added markdown files

Would this be different from any other time we move files around? Seems like if A redirects to B then we move B to a new page C, we need to add redirects from B to C regardless, and the chain A -> B -> C would work. Sorry if I'm misunderstanding something.

@bvaughn
Copy link
Contributor Author

bvaughn commented Oct 4, 2017

Would this be different from any other time we move files around?

Yes, in that it would be unexpected. As someone modifying markdown content in a *.md file, I wouldn't expect to know that I should also go hunt down some JavaScript redirect in a different part of the website repo to keep its #anchortaglikethis in sync with my # Markdown Title Like This.

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

Successfully merging this pull request may close these issues.

Active Tab is not included in current route or permalinks on new docs site
5 participants