-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix lampepfl/dotty-feature-requests#28: make Dottydoc responsive #7153
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Commit Messages
We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).
Please stick to these guidelines for commit messages:
- Separate subject from body with a blank line
- When fixing an issue, start your commit message with
Fix #<ISSUE-NBR>:
- Limit the subject line to 72 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line ("Add" instead of "Added")
- Wrap the body at 80 characters
- Use the body to explain what and why vs. how
adapted from https://chris.beams.io/posts/git-commit
Have an awesome day! ☀️
d4ca271
to
5655153
Compare
rebased on master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work @TheElectronWill, thanks!
I’ve had a quick look to the diff and the changes look good to me except for the usage of const
and let
, which might be incompatible with old browsers.
var insertMember = function(member, li) { | ||
var div = document.createElement("div"); | ||
var insertMember = function(member, li, parentLink) { | ||
const div = document.createElement("div"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, by using const
we might make the website unreadable on some old browsers: https://caniuse.com/#feat=const. I would suggest reverting to div
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean reverting to var
? I've reverted to var
.
@@ -85,7 +85,7 @@ trait MarkupConversion[T] extends MemberLookup { | |||
short = stringToShortHtml(parsed.body), | |||
authors = filterEmpty(parsed.authors).map(markupToHtml), | |||
see = filterEmpty(parsed.see).map(markupToHtml), | |||
result = single("@result", parsed.result).map(markupToHtml), | |||
result = single("@return", parsed.result).map(markupToHtml), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
Is it possible to workaround this issue by converting the |
7acbee1
to
c92e0a5
Compare
I've just tried to convert the |
I just compiled the website locally and it looks great! However the pages seam to glitch quite a bit until they are fully loaded, do you have any idea how to fix that? Here is a video illustrating what I'm seeing on my machine: 2019-09-06 14-52-02.flv.zip |
@OlivierBlanvillain Can reproduce on Chrome, thank you for letting me know! For some reason there is much less glitching on Firefox... I think there are multiple factors here:
I should be able to solve point 3 and to improve point 1. For point 2 I'll take a closer look at what hljs does and adjust the css and/or reconfigure hljs. |
0e971b0
to
e17d964
Compare
@OlivierBlanvillain @julienrf I've fixed the aforementioned issues. Chrome still flashes when navigating the local files but it's fluid with a web server. I've also fixed the code blocks that have been broken by fec2ae5. Basically, this markdown syntax to create code blocks: def someCode = ... // indented with 4 spaces is now unsupported and must be replaced by: ```scala
def someCode = ...
``` The replacements have been done in e17d964. This PR now looks complete 😃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I'll merge after resolution of conflicts on docs pages.
Thanks a lot for the PR, it looks like a lot of work went into this and the result is a really nice improvement over the current state.
SimpleType ::= ... | ||
| ‘$’ ‘{’ Block ‘}’ | ||
|
||
```none |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we gain anything by using none
instead of just leaving it empty? If no I would rather not have it as it adds noise when reading the md files directly...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If no language is declared, hljs will try to guess it, which leads to incorrect and annoying highlighting, especially for ASCII graphs or syntax definitions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've found a better solution: 685f906
Leaving it empty now works as expected, ie it disables highlighting.
@OlivierBlanvillain conflict resolved |
c8dc3f5
to
6925f4c
Compare
ping @OlivierBlanvillain before more conflicts show up. I've replaced the previous merges by a much cleaner rebase. |
The conflicts are coming in faster than the time it takes for a CI build to complete, sorry about that... If you're tiered of rebasing the entire thing you could also try isolating the superficial changes too docs into separate PR. |
6925f4c
to
3ace390
Compare
Rebasing is actually pretty fast and I don't mind doing it multiple times :) |
An excessive ".." in baseUrl caused errors when using the generated website locally
With bootstrap, popper should be used instead of tether. If required the bootstrap bundle can be added later. See https://getbootstrap.com/docs/4.3/getting-started/introduction/#js
FontAwesome: 4.7.0 -> 5.9.0 Bootstrap: 4.0.0-alpha5 -> 4.3.1 jQuery: 3.1.1 -> 3.4.1 hljs: 9.9.0 -> 9.15.9 Note: fontawesome's css requires several fonts so it seems useless to put the .css alone in the resources. Note 2: some (unused and unlikely to be used) languages have been removed from highlight.pack.js. They can be added as needed in javascript (see https://highlightjs.readthedocs.io/en/latest/api.html#registerlanguage-name-language)
- add plain scala logo - allow caching - prevent css modifications
The condition `{% if site.projectUrl %}` only filters out `null` values, not empty ones.
For instance, in `reference/other-new-features`, `opaques-details.html` doesn't appear in the sidebar but is "linked" to `opaques.html` and therefore should expand the same hierarchy.
A previous commit disabled indentation-based code blocks because it's incompatible with liquid template inclusion. This commit replaces all indented code blocks by backticks-surrounded code blocks. It also adds missing language declarations (for syntax highlighting) and removes some annoying extra indentation.
3ace390
to
052c2cf
Compare
2102ca1
to
685f906
Compare
@OlivierBlanvillain all green |
Awesome contribution, thank you! ✨ The blog post for the 0.19.0-RC1 release is currently being written at #7269, would you mind contributing a section on the dottydoc redesign ? (since you can't push to the PR, you can just send a markdown comment)
Unfortunately not, we cannot allow external scala dependencies in the dotty project because we want to be able to break binary compatibility, which could break these dependencies. This could be possible if we made dottydoc an independent project instead of a part of the compiler, this is complicated right now because dottydoc uses internal compiler APIs. To fix this a proof of concept replacement for dottydoc has been developed: https://github.com/BryanAbate/dotty/tree/tastydoc/tastydoc. Tastydoc takes .tasty files as input (which means it should be much faster since it doesn't have to run the typechecker!) and currently generates .md, but should be extended to generate HTML. It's implemented using tasty-reflect, the same API used to write macros, this should make it possible to develop it independently of the compiler. No one is actively working on tastydoc/dottydoc currently, so if you're interested in contributing more, we'd be more than happy to guide you ! |
I've written a small section for the blog post.
Very interesting, using TASTy instead of the compiler internals seems future-proof. I'll take a look at the APIs. Is tasty-reflection-exercise up-do-date?
I'll be happy to contribute more! IIUC, tastydoc is intended to eventually replace dottydoc, so I should focus on tastydoc? |
I don't think so, @nicolasstucki can you point @TheElectronWill to up-to-date tasty-reflect resources ?
I think that's a good idea yes.
We could do that yes, what do you think @AleksanderBG ? |
I will try to update the exercise soon. The best examples are probably in the community build
|
Thank you! I'll look at these examples and play with TASTy to get used to it. |
@smarter @TheElectronWill Tastydoc is tied to Dotty's build, so I'm not sure if having it in a separate repo is the best approach. Last version can be found here https://github.com/BryanAbate/dotty/tree/tastydoc. Since AFAIU the entire codebase is in the |
Sounds good to me. |
LGTM too |
I know this PR is 2 month old but I just fell upon it now, I hope it does not bother anyone I am commenting now. @TheElectronWill (and @AleksanderBG ) I don't know if you are still working on this but if you need any help with my proof of concept for Tastydoc I am more than happy to help. Also you should not need to understand how Tasty works to produce html as I made sure to offer a complete API on top of Tasty especially for people who want to work on a doctool without worrying to understand how Tasty works. You just need to call the TastyConsumer and then you are left with the representation API: https://github.com/BryanAbate/dotty/blob/tastydoc/tastydoc/src/dotty/tastydoc/representations.scala. You can give a look to the main class for a complete example. Also, more oriented to Alex, if you need help with integration/something else, I am still more than happy to help to be able to merge the work with dotty. |
Thank you @BryanAbate! I didn't have time to work on this since September but it's still on my TODO list. It's great that you've made an API that abstracts Tastydoc! That will make things easier, I'll definitely check it out. |
This PR implements an overhaul of the Dottydoc website in order to make it responsive. This is mostly front-end work, very few scala files have been modified.
Bootstrap theme
The new design is based on a customized version of the Bootstrap framework, located in the
bootstrap-theme
folder. This has several advantages:Downside: the inclusion of the long file
package-lock.json
(4500+ lines!) makes it look like this PR increases the size and the complexity of the codebase, whereas many html and css files have actually been simplified 😃Scala changes
In addition to some tweaks to the page generation code, I've added a new option
-project-logo
to easily change the toolbar's logo without touching any code.Visual changes
The most visible changes are the toolbar and sidebar, which have been reworked. Also, the fonts have been harmonized across all the elements.
Below are some screenshots of the updated Dottydoc website.
Front page
Let’s start small. Updating Boostrap broke the front page menu. I fixed it and took the opportunity to simplify the code and improve the fonts a bit.
Before
After
I can do something about the "scroll down for more info" but that will be another PR 😃
Blog page
Before
and
After
and
Sidebar and toolbar
The sidebar is now bigger, more readable and toggleable. The toolbar is a bit smaller and has a better contrast.
Here is what it looks like on desktop:
Here is what it looks like on small screens:
API Search
The search page is responsive too! And it generates links to the relevant members.
Before
After
API documentation
Of course the documentation looks better on desktop than on mobile. But it's still possible to do nice things:
Before
After
Unfortunately I’ve been limited by liqp, which (among others) doesn’t support iterating on Maps. More content to the API pages could be added once Dottydoc has a more powerful Liquid engine.
There is a feature request for implementing Liquid inside of Dottydoc. I wonder if it should be a separate library instead, since it could be useful to other projects. Is it possible to make Dottydoc depend on an external dotty library?