-
Notifications
You must be signed in to change notification settings - Fork 53
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
Make HTML output look and feel better #130
Comments
A non-exhaustive list of good ideas from the examples above and improvements that probably also would be good for llsoftsecbook are:
|
A first set of improvements landed in #135 |
The new PR looks great! Now that I had a chance to use it interactively, I just had two thoughts.
edit: Just found these which might be a more bookdown idiomatic way of changing this: 1 and 2 happy to investigate a PR for either/both of these issues if helpful. |
Another thing I noticed testing on mobile is that the hamburger button doesn't work with JS disabled. I think this probably OK but thought it worth mentioning as we had previously discussed to what extent the book should cater to non-JS book viewers in #64 |
Thank you for trying out the new design, @JLouisKaplan-Arm !
I also noticed this morning that the scroll bar shifts left when removing the TOC sidebar on large screens. I think that definitely needs to get fixed. With bootstrap, I think it should be trivial to implement hiding the hamburger icon on large screens only by adding the appropriate bootstrap class to it. But as said above, I think it's better to keep giving the user the option to hide the sidebar also on large screens.
I'd be delighted if you would create a PR for the second issue you see. |
Yes, I could not find a way to make the hamburger work as desired without using a bit a javascript, I'm afraid. |
Yes that makes sense to me. Definitely nice to keep the option if possible.
Sounds good - will try and give it a go.
It is possible with a fair amount of CSS hacking (explored this a while ago for a personal website) but I have no idea if it will be compatible with the rest of the bookstrap approach. Personally I think the featurefullness of JS makes using it here worthwhile so I'm happy as it is. |
I just put up #136 to try to fix the position-of-scrollbar issue on large screens discussed above. |
A few more thoughts after trialling the new UI
I'm really being fussy here though. I think it looks fantastic already. |
I'm not fully sure, but I think you mean that if you make the TOC invisible, the main text does not move to a different position? Currently, the text stays centered on the space it has available on the screen, which is less if the TOC is visible.
Yes, we should. Let me add a "todo" on a higher up comment which lists open todos.
Agreed, this has annoyed me too. I've tried to (partially) address this in #141.
Thank you for being fussy. I think we need to be fussy to make it look fantastic ;) |
Ah right I see now that it does behave that way. I was viewing parts of the book without any sidenotes (and with TOC disabled) so it seemed like the main text was aligned off-center slightly to the left. But now seeing the sections with side notes I see that it is centered when considering (text + side notes) as a column altogether. Hope that makes sense. As for one or the other approach, I have no preference :) A last thing I noticed today - is it expected that when first landing on the page (before clicking any sections or toggling TOC) and scrolling down, that there is a floating vertical scroll bar for the main content? (I.e. it's not 'attached' to the edge of the viewport.) This seems to disappear with a toggle/re-toggle of the TOC, after which the vertical scroll bar moves to where I would expect it at the rightmost edge of the viewport. |
I also noticed a similar issue when first opening the page in Chrome yesterday. On a large screen, I noticed the width of the sidebar and the main text somehow being wider than the viewport, and that resulting in seeing the TOC sidebar centered, and below it, if you scroll, the main text. Not sure if it's the same issue as what you're seeing. I'll try to do a few experiments today to see where the issues shows up (is it browser-specific? OS-specific? does it happen at all screen sizes?) Update: I now tried it with a number of browsers on Windows, OSX and linux. It seems this issue only shows up on Windows, across all browsers I tried (Firefox, Chrome, Edge). |
A bit late to the party, but I'm wondering if we need the tooltips at all on mobile. To make them appear you need to already click on the button. I don't particularly mind the tooltips for the repo button and "suggest an edit" button, even though they will only appear shortly after you click on them, before the browser navigates to their target, and then again if you navigate back (when they are "sticky"). But the TOC tooltip is sticky even though you press the button which has clearly toggled the TOC (so the tooltip doesn't give any new information), and keeps hiding the other two buttons. To get rid of it, you need to click somewhere else. WDYT? |
Yeah, not having the tooltip on interfaces where you use fingers rather than a mouse make sense to me. |
Maybe something to change in javascript instead? Where we initialise tooltips, for example:
|
There's a PR for this now at #144 . |
There's now a PR for adding a download button at #235 |
The current HTML version of the book can be improved a lot.
In this issue, let's record an overview of all the ideas for improvements. More detailed discussions for specific improvements can be done in separate issues when that makes sense.
Overall, there are a number of examples of HTML books that show us examples of how nice and functional HTML books can look. Let's take inspiration from them. At the moment, I think we have at least 3 examples:
The text was updated successfully, but these errors were encountered: