Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Broken Sidebar on website API for master #15200

Closed
ThomasDelteil opened this issue Jun 10, 2019 · 20 comments
Closed

Broken Sidebar on website API for master #15200

ThomasDelteil opened this issue Jun 10, 2019 · 20 comments

Comments

@ThomasDelteil
Copy link
Contributor

See https://mxnet.incubator.apache.org/versions/master/api/python/index.html

Screen Shot 2019-06-10 at 3 56 20 PM

vs

Screen Shot 2019-06-10 at 3 56 55 PM

@mxnet-label-bot
Copy link
Contributor

Hey, this is the MXNet Label Bot.
Thank you for submitting the issue! I will try and suggest some labels so that the appropriate MXNet community members can help resolve it.
Here are my recommended labels: Doc

@ThomasDelteil
Copy link
Contributor Author

@aaronmarkham

@aaronmarkham
Copy link
Contributor

Bad news
I tested a variety of build configurations.

  1. In settings, I turned the artifacts feature on and off.
  2. I built all versions plus using 1.5.x to see if a recent cut had a similar problem. It does not. 1.5.x looks fine.
  3. I built a variation of master with a different name and the nav is broken.

Good news
I tested changing the default version to 1.4.1 and it works or see the root working and test the dropdown.

@aaronmarkham
Copy link
Contributor

I tried rolling back to some older commits and found the site to work.
Then after trying many older commits I decided to look at the s3 previews coming out of CI for anything in /doc history. This is very strange - the previews have broken navs then working navs in no obvious order.

  1. broken: http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-14987/8/api/python/index.html
  2. working: http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-15111/2/api/python/index.html
  3. working: http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-15156/4/api/python/index.html
  4. working: http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-15185/1/api/python/index.html
  5. broken: http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-15205/1/api/python/index.html

Maybe something was going on with CI (and still is?).

@larroy
Copy link
Contributor

larroy commented Jun 13, 2019

Have you tried to rebuild one of those commits locally?

@aaronmarkham
Copy link
Contributor

aaronmarkham commented Jun 13, 2019

I built every commit backwards until I narrowed it down to this one:
#14987

Should I revert this commit? I can't see how it would be causing the issues, but the commit just prior comes out ok.

@aaronmarkham
Copy link
Contributor

I noticed that the content is in the page, but not visible. I don't know why this bug would only affect master, but I imagine the solution is in the javascript that moves the sidebars around.
https://github.com/apache/incubator-mxnet/blob/master/docs/_static/js/sidebar.js

This area of code is the source of several bugs - from the footer overlapping content when the page toc is too short to not opening the page you expect.

I pointed the website default to 1.4.1 to lessen the disruption for users for now. I don't think I should do a revert on the commit I narrowed down to because the changes wouldn't have this kind of impact. It has to be something else.

I think we need someone to focus on the core issue - the js hacks on the theme and try to fix those directly.

@larroy
Copy link
Contributor

larroy commented Jun 13, 2019

Could it be the changes from HTML to md in the links?

@larroy
Copy link
Contributor

larroy commented Jun 13, 2019

Can you try to use the browser Dev tools and debug keepExpand to see why the links are not generated? Could be that they don't come from sphynx from a quick look I didn't see anything obvious with the markdown vs HTML.

@aaronmarkham aaronmarkham removed the Doc label Jun 13, 2019
@aaronmarkham
Copy link
Contributor

Yes. It seems like there are quite a few bugs in there - variables that should be tracking what's an API page that are undefined. Seems to be less a doc problem and more a front-end engineering problem to me.

@ChaiBapchya
Copy link
Contributor

@aaronmarkham

I noticed that the content is in the page, but not visible

Well. I tried to compare the working V1.4.1 vs master
Here's the console output

Ideal Condition. V1.4.1

V1.4.1 Console for sidebar

Master

Div <div class="sphinxsidebar leftsidebar"> exists. But the content is missing.

Screen Shot 2019-07-26 at 2 04 48 PM

Any idea what am I missing?

@larroy
Copy link
Contributor

larroy commented Jul 26, 2019

I don't know much about sphinx.

https://github.com/apache/incubator-mxnet/blob/master/docs/_static/mxnet-theme/layout.html

This seems to reference localtoc: https://github.com/sphinx-doc/sphinx/blob/master/sphinx/themes/basic/localtoc.html

Which seems to be missing. So I guess the toc is not being generated correctly. Let me generate the docs and see if there's some error message with the TOC.

@larroy
Copy link
Contributor

larroy commented Jul 26, 2019

I think the problem is in sidebar.js that component is loaded dynamically in JS. It has to be debugged with the browser dev tools. I would recommend side to side debugging with 1.4 and 1.5 to find out what's the problem.

A breakpoint in render_lefttoc or render_left_helper... from sidebar.js

@larroy
Copy link
Contributor

larroy commented Jul 27, 2019

I think the problem is in the generated html,is missing the element ul class "current" so the selector gets an empty text. Looks like generation problem.

@larroy
Copy link
Contributor

larroy commented Jul 27, 2019

It's quite puzzling, I have look at many places, compared with 1.4.1 and don't find the root cause.

We are using the same sphinx version, the templates are not changed... but somehow the left sidebar is not generated properly.

@ChaiBapchya
Copy link
Contributor

Yup. Went through the steps you mentioned, (breakpoints, comparison with previous version, debug)
But missing the root cause..

@larroy
Copy link
Contributor

larroy commented Jul 29, 2019

We need to bisect, to find out when it was broken. I have a script to do that.

@aaronmarkham
Copy link
Contributor

@larroy Not sure if it is worth the effort. We're hoping to launch a refactored website soon and we won't be using any of this old js or sphinx theme. But, if it's easy and you want to try... don't let me stop you!

@larroy
Copy link
Contributor

larroy commented Jul 29, 2019

OK, I have other things to do, I thought this was important for the release.

@aaronmarkham
Copy link
Contributor

New site makes this a moot issue.

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

No branches or pull requests

5 participants