responsive side navigation #449

wookay · 2017-03-16T23:11:38Z

It's too hard to read the docs on mobile,
I did just make nav.toc to be relative.

Here's an example.
https://wookay.github.io/Documentery/html/

thanks.

mortenpi · 2017-03-20T10:24:00Z

Thanks for tackling this. Mobile UX is certainly somethings that's on the todolist. A couple of things that I noticed:

We can probably have it be 100% wide on mobile screens, and also drop the shadow thing on the right. I.e. that the menu would cover the whole screen
How hard do you think it would be to have the page header show up when the user pulls the page down? I.e. that you wouldn't have to scroll all the way up to open the menu. E.g. something like the Material theme does for MkDocs.

wookay · 2017-03-21T23:42:07Z

We can probably have it be 100% wide on mobile screens, and also drop the shadow thing on the right. I.e. that the menu would cover the whole screen

it's not perfect, added the topbar. see the Julia Documentation example.
https://wookay.github.io/Documentery/en/

How hard do you think it would be to have the page header show up when the user pulls the page down? I.e. that you wouldn't have to scroll all the way up to open the menu. E.g. something like the Material theme does for MkDocs.

then we need more javascript framework like bootstrap. mkdocs looks really good.

randy3k · 2017-04-11T15:02:27Z

It looks very good. Is there any reason of not merging this (besides the merge conflict)?

mortenpi · 2017-04-17T00:17:43Z

Sorry, I just haven't gotten around to reviewing this. I'll try to go through this in the next few days, hopefully to get this released as part of 0.10. Together with #409 this will be a nice set of updates to the frontend bit.

mortenpi · 2017-04-19T08:42:53Z

Playing around with it now I quite like the top bar and the ~80% width of the menu. But I do have a couple of items of feedback, so I hope you would be up for iterating on this PR a bit.

Clicking on (touching?) the main page behind the menu should probably close the menu (e.g. how it's done here)
If the page is wider than the screen then it breaks a bit, e.g. on the Examples page.
Also, if the screen is very wide (e.g. phone in landscape), I feel that the menu probably shouldn't extend to 80%, but only be as wide as the text basically (e.g. the Material theme).
The .topbar actually duplicates the <header> tag. Easiest could be to just hide the header in mobile mode. Or alternatively use the header tag for both things. I think it's fine if we drop the "Edit on GitHub" link on mobiles.
If you're scrolled down and you click on the menu button (), it jumps back to the beginning of the page. Probably would be nice if wouldn't do that.
The menu seems to scroll together with the page even if the menu is hidden. Would better if it didn't I think, since in practice this means that you are at the bottom of the menu every time you open it up.

mortenpi · 2017-04-19T08:03:02Z

src/Writers/HTMLWriter.jl

    navmenu = render_navmenu(ctx, navnode)
    article = render_article(ctx, navnode)

    htmldoc = DOM.HTMLDocument(
        html[:lang=>"en"](
            head,
-            body(navmenu, article)
+            body(topbar, div[".main"](div[".container"](navmenu, article)))


I would prefer avoiding unnecessary divs. They're not used in the style, so as far as I can tell they could be dropped. A body(topbar, navmenu, article) ought to do.

mortenpi · 2017-04-19T08:29:29Z

assets/html/documenter.css

+        position: fixed;
+        top: 0px;
+        left: 0px;
+        right: 0px;


Should drop the units from the zeroes here.

mortenpi · 2017-04-19T08:37:22Z

src/Writers/HTMLWriter.jl

+    page_title = string(mdflatten(pagetitle(ctx, navnode)))
+    topbarleft = ul[".float-left"](li(span(page_title)))
+    topbarright = ul[".float-right"](li(a[".btn", :href => "#"](i[".fa .fa-bars", Symbol("aria-hidden") => "true"])))
+    div[".topbar"](div[".container"](topbarleft, topbarright))


As above -- any reason we couldn't just drop the .container <div>?

mortenpi · 2017-04-19T08:39:52Z

src/Writers/HTMLWriter.jl

+
+    page_title = string(mdflatten(pagetitle(ctx, navnode)))
+    topbarleft = ul[".float-left"](li(span(page_title)))
+    topbarright = ul[".float-right"](li(a[".btn", :href => "#"](i[".fa .fa-bars", Symbol("aria-hidden") => "true"])))


Why is the aria-hidden here? Based on this article it seems that aria-hidden is not very useful and, if anything, we should use HTML5's hidden attribute.

Also, why use uls here, this does not feel like a list to me? A simple <div id="topbar">{PAGE_TITLE}<a class="btn fa fa-bars"></a></div> ought to do the trick here.

just updated, used span tag to align the PAGE_TITLE in the css.

wookay · 2017-04-19T16:22:42Z

thanks for your reviews! I would investigate it at tomorrow.

wookay · 2017-04-20T16:33:12Z

Fixed almost stuff,
this pr requires headroom.js (http://wicky.nillia.ms/headroom.js)

mortenpi

Thanks for the updates! It's a really nice UX improvement on mobiles. I think headroom is worth the additional dependency.

Just a few last things:

I still feel that the article > header and #topbar duplicate each other. Maybe we should just have a position: fixed on article > header, hide the edit link <span> and show the button when in mobile mode?
I am also wondering whether it would make sense to keep the subsection titles even on mobile (i.e. like in article > header).

mortenpi · 2017-04-25T07:24:26Z

src/Writers/HTMLWriter.jl

+    @tags div span a
+
+    page_title = string(mdflatten(pagetitle(ctx, navnode)))
+    div["#topbar"](span(page_title), a[".btn .fa .fa-bars", :href => "#"])


I don't think we have a .btn class, so no need to apply it here I'd say.

mortenpi · 2017-04-25T08:10:40Z

assets/html/documenter.css

+        position: fixed;
+        width: 100%;
+        height: 2.7em;
+        background-color: white;


I would also give the #topbar a slight background color as well, probably the same as the menu #fcfcfc.

mortenpi · 2017-04-25T08:21:46Z

assets/html/documenter.css

+        display: block; /* is mobile */
+        position: fixed;
+        width: 100%;
+        height: 2.7em;


I would make the #topbar a bit wider in the vertical direction.

If I may I would propose a way to set the vertical heights in the following way, which I think would be a bit more clear:

Have padding-top: 1em and padding-bottom: 1em on the #topbar.

Drop margin-top from div#topbar span and only keep padding-right: 1em for div#topbar a.fa-bars

Have an explicit font-size: 1.5em on the div#topbar a.fa-bars (as opposed x-large) -- no other reason than to have an explicit number here.

The navlink text will be smaller, so for the #topbar it should also be height: 1.5em.

The total height of the #topbar will then be 3.5em, so we should also have a padding-top: 3em on the article, since the default 2em will not be enough.

The headroom should also then probably be something like top: -4em.

mortenpi · 2017-04-25T08:26:16Z

assets/html/documenter.css

+        display: none;
+    }
+
+    article [id]:before {


Confused a bit here -- why do we need this one?

oh, it doesn't need anymore, I'll fix.

it was used to prepend the spacing of the article.
when touching the url with the fragment identifier such as
https://wookay.github.io/Documentery/en/manual/strings.html#man-characters-1
unfortunately, sometimes h2 anchor gets hidden over by topbar.

mortenpi · 2017-04-25T08:29:02Z

assets/html/documenter.css

+        z-index: 1;
+    }
+
+    article table {


Also, why do we have this one exactly? Maybe you could add a few comments here to explain why/how we need to give some elements special treatment to make sure they render properly on mobile (i.e. so that someone not familiar with mobile UX could get up to speed quickly).

article table { table-layout: fixed; width: 99%; }

used this to fix the table overflowing such as
https://wookay.github.io/Documentery/en/stdlib/linalg.html#Base.LinAlg.qrfact

wookay · 2017-04-25T16:26:57Z

thanks a lot for the details!
I have applied above reviews, and

move the div#topbar in the article header.
a.fa-bars has padding around 0.6em to enlarge the touch area.
media screen max-width 320px's body has the font-size: 15px, because it has a bit different for the iPhone SE on the Safari's Responside Design Mode.

wookay · 2017-05-03T04:23:55Z

I just found a tool, Sizzy (https://github.com/kitze/sizzy)
here are links to browse around.
https://sizzy.co/?url=https%3A%2F%2Fwookay.github.io%2FDocumentery%2Fen%2F
https://sizzy.co/?url=https%3A%2F%2Fwookay.github.io%2FDocumentery%2Fhtml%2F

jebej · 2017-05-06T14:45:02Z

Is this ready to go?

mortenpi

Everything LGTM now, so could be merged now. Just one final thing about tables.

mortenpi · 2017-05-07T04:49:12Z

assets/html/documenter.css

+        color: #3091d1;
+    }
+
+    article table {


Tables that should be wide become quite ugly on narrower screens at the moment (e.g. https://sizzy.co/?url=https%3A%2F%2Fwookay.github.io%2FDocumentery%2Fen%2Fstdlib%2Flinalg.html%23Base.LinAlg.qrfact).

Perhaps having overflow-x: auto; display: block; on a table would be better? This should allow the user to scroll wide tables instead. Any thoughts on this?

updated that table styles, it looks much better.

wookay · 2017-05-07T06:54:07Z

rebased past commits :)

KristofferC · 2017-05-07T10:18:32Z

This is awesome!

mortenpi · 2017-05-07T10:18:46Z

Thank you very much for the contribution, for catering to all my requests and being patient with my slow reviews, @wookay! I think it's a really great addition and we can now finally strike this long standing item off the todo list 😄

wookay · 2017-05-07T15:51:53Z

you're welcome. it is really appreciated at all.
I'm happy to help the improvement.

wookay force-pushed the woo/mobile_css branch from d4d49a8 to bd134c5 Compare March 17, 2017 06:28

mortenpi added this to the 0.10 milestone Mar 20, 2017

mortenpi assigned mortenpi and unassigned mortenpi Mar 20, 2017

mortenpi added the Type: Enhancement label Mar 20, 2017

wookay force-pushed the woo/mobile_css branch from 31a18a5 to d868862 Compare March 21, 2017 23:25

wookay force-pushed the woo/mobile_css branch from d868862 to 8abc224 Compare March 22, 2017 10:39

wookay force-pushed the woo/mobile_css branch from 8abc224 to 1f7f090 Compare April 17, 2017 03:06

mortenpi reviewed Apr 19, 2017

View reviewed changes

mortenpi modified the milestones: 0.10, 0.11 Apr 19, 2017

wookay force-pushed the woo/mobile_css branch from fc3726c to 235a716 Compare April 20, 2017 16:24

wookay force-pushed the woo/mobile_css branch from 235a716 to 6ac7b95 Compare April 20, 2017 18:13

mortenpi requested changes Apr 25, 2017

View reviewed changes

wookay force-pushed the woo/mobile_css branch 2 times, most recently from fde237d to bbd8c69 Compare April 28, 2017 12:34

mortenpi approved these changes May 7, 2017

View reviewed changes

mortenpi reviewed May 7, 2017

View reviewed changes

Responsive side navigation

afec9a7

wookay force-pushed the woo/mobile_css branch from 53a4db0 to afec9a7 Compare May 7, 2017 06:41

mortenpi merged commit 193a9dc into JuliaDocs:master May 7, 2017

mortenpi mentioned this pull request May 7, 2017

Meta: improvements to the HTML output #212

Open

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

responsive side navigation #449

responsive side navigation #449

wookay commented Mar 16, 2017

mortenpi commented Mar 20, 2017

wookay commented Mar 21, 2017

randy3k commented Apr 11, 2017

mortenpi commented Apr 17, 2017

mortenpi commented Apr 19, 2017 •

edited

Loading

mortenpi Apr 19, 2017

mortenpi Apr 19, 2017

mortenpi Apr 19, 2017

mortenpi Apr 19, 2017

wookay Apr 20, 2017 •

edited

Loading

wookay commented Apr 19, 2017 •

edited

Loading

wookay commented Apr 20, 2017 •

edited

Loading

mortenpi left a comment

mortenpi Apr 25, 2017

mortenpi Apr 25, 2017

mortenpi Apr 25, 2017

mortenpi Apr 25, 2017

wookay Apr 25, 2017

wookay May 3, 2017

mortenpi Apr 25, 2017

wookay Apr 25, 2017

wookay commented Apr 25, 2017 •

edited

Loading

wookay commented May 3, 2017

jebej commented May 6, 2017

mortenpi left a comment

mortenpi May 7, 2017

wookay May 7, 2017

wookay commented May 7, 2017

KristofferC commented May 7, 2017

mortenpi commented May 7, 2017

wookay commented May 7, 2017

responsive side navigation #449

responsive side navigation #449

Conversation

wookay commented Mar 16, 2017

mortenpi commented Mar 20, 2017

wookay commented Mar 21, 2017

randy3k commented Apr 11, 2017

mortenpi commented Apr 17, 2017

mortenpi commented Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wookay Apr 20, 2017 • edited Loading

Choose a reason for hiding this comment

wookay commented Apr 19, 2017 • edited Loading

wookay commented Apr 20, 2017 • edited Loading

mortenpi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wookay commented Apr 25, 2017 • edited Loading

wookay commented May 3, 2017

jebej commented May 6, 2017

mortenpi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wookay commented May 7, 2017

KristofferC commented May 7, 2017

mortenpi commented May 7, 2017

wookay commented May 7, 2017

mortenpi commented Apr 19, 2017 •

edited

Loading

wookay Apr 20, 2017 •

edited

Loading

wookay commented Apr 19, 2017 •

edited

Loading

wookay commented Apr 20, 2017 •

edited

Loading

wookay commented Apr 25, 2017 •

edited

Loading