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

nav: left sidebar isn't visible without js #856

Closed
jorgeorpinel opened this issue Dec 11, 2019 · 10 comments · Fixed by #991
Closed

nav: left sidebar isn't visible without js #856

jorgeorpinel opened this issue Dec 11, 2019 · 10 comments · Fixed by #991
Labels
type: enhancement Something is not clear, small updates, improvement suggestions website: eng-doc DEPRECATED JS engine for /doc

Comments

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Dec 11, 2019

Disclaimer: I'm not sure the title is completely accurate. See #834 (review) for context Summary:

J: without js the nav bar doesn't expand
I: The good question is if we need to generate a sitemap or something for the case when we don't have a link to some page inside and search engines won't be able to find it?
J: maybe it's easy to pre-render nav?

So, possible solutions:

  1. Pre-render all nav elements
  2. Introduce a (automatic) site map.
@jorgeorpinel
Copy link
Contributor Author

Thoughts @iAdramelk?

@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) website: eng-doc DEPRECATED JS engine for /doc A: website Area: website labels Dec 11, 2019
@jorgeorpinel jorgeorpinel mentioned this issue Dec 11, 2019
@iAdramelk
Copy link
Contributor

iAdramelk commented Dec 12, 2019

@jorgeorpinel Not exactly. NextLink is wrapping the regular links and passing the href for them. So in the generated HTML, there are always exists regular links with the correct href attribute. On the client navigation through this links is prevented by Event.preventDefault(), and Next's logic is used instead, but crawlers and users can just use regular links and hrefs as usual.

@iAdramelk
Copy link
Contributor

But the submenus that are not visible without JS is a problem. We probably should fix it together with #724. And it means that we should remove the need for set height to display submenu, so either remove animations or implement them in some other way.

@jorgeorpinel
Copy link
Contributor Author

So in the generated HTML, there are always exists regular links with the correct href attribute.

So you're saying the SSR HTML does have the full nav regardless of JS? (Meaning the site can be easily crawled.)

But the submenus that are not visible without JS is a problem...

Agree but would not combine that with #724. Would just change the title and description of this issue.

@shcheklein
Copy link
Member

Just to be precise - the title is not exactly correct, btw - most engines crawl the website easily since they support JS. So, it's more about optimization, including SEO of course. And SSR itself is more about proper behavior with things like 404 and optimization again.

@shcheklein
Copy link
Member

shcheklein commented Dec 12, 2019

also, NextLink is "crawlable", the problem is with nav bar only as far as I understand (as it was before btw without SSR).

@jorgeorpinel jorgeorpinel changed the title nav: search engines can't crawl the site because NextLink's don't SSR nav: NextLink's don't SSR Dec 16, 2019
@jorgeorpinel jorgeorpinel changed the title nav: NextLink's don't SSR nav: NextLink's don't SSR. Is this a problem? Dec 16, 2019
@jorgeorpinel jorgeorpinel changed the title nav: NextLink's don't SSR. Is this a problem? nav: left sidebar doesn't SSR. Is this a problem? Dec 16, 2019
@jorgeorpinel jorgeorpinel changed the title nav: left sidebar doesn't SSR. Is this a problem? nav: left sidebar doesn't SSR. Dec 16, 2019
@jorgeorpinel jorgeorpinel added type: enhancement Something is not clear, small updates, improvement suggestions and removed A: website Area: website labels Dec 29, 2019
@jorgeorpinel
Copy link
Contributor Author

submenus that are not visible without JS is a problem.

@iAdramelk are they just not visible but the HTML is there? Because in that case this may not be a problem after all.

@iAdramelk
Copy link
Contributor

@jorgeorpinel yep, just not visible. Because we can't calculate submenus height correctly on the server, we just assign them height 0. But it still would be a good thing to have working menus even without the js.

@jorgeorpinel jorgeorpinel changed the title nav: left sidebar doesn't SSR. engine: left sidebar isn't visible without js Dec 29, 2019
@jorgeorpinel jorgeorpinel changed the title engine: left sidebar isn't visible without js engine: nav (left sidebar) isn't visible without js Dec 29, 2019
@jorgeorpinel
Copy link
Contributor Author

OK so they ARE rendered, but with height 0. I then agree with Alexey's idea:

We probably should fix it together with #724. And it means that we should remove the need for set height to display submenu, so either remove animations or implement them in some other way.

Do you agree @shcheklein ? If so I'll move this requirement over to that existing issue.

@shcheklein
Copy link
Member

@jorgeorpinel I would keep them separate. They are too different. It just happened that a PR for that one can close this one as well.

@jorgeorpinel jorgeorpinel changed the title engine: nav (left sidebar) isn't visible without js docs: nav (left sidebar) isn't visible without js Jan 5, 2020
@jorgeorpinel jorgeorpinel changed the title docs: nav (left sidebar) isn't visible without js ui: nav (left sidebar) isn't visible without js Jan 10, 2020
@jorgeorpinel jorgeorpinel removed the A: docs Area: user documentation (gatsby-theme-iterative) label Jan 18, 2020
@jorgeorpinel jorgeorpinel added the 🐛 type: bug Something isn't working. label Jan 18, 2020
@jorgeorpinel jorgeorpinel changed the title ui: nav (left sidebar) isn't visible without js nav: left sidebar isn't visible without js Jan 18, 2020
@jorgeorpinel jorgeorpinel removed the 🐛 type: bug Something isn't working. label Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Something is not clear, small updates, improvement suggestions website: eng-doc DEPRECATED JS engine for /doc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants