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

Global navigation bar #86

Merged
merged 4 commits into from
Sep 10, 2021
Merged

Global navigation bar #86

merged 4 commits into from
Sep 10, 2021

Conversation

TheLortex
Copy link
Contributor

Supersedes #48

Now that voodoo generates a package.json file for each project containing the module map, we can implement a global navigation bar.

The first commit parses the package.json, the second commit use the data structure to generate a navigation bar in the side panel along with the table of contents.

This is a first iteration as a proof of concept, work on the design is needed. Moreover I had to remove the fixed class because the two sidebars were overlapping and were too long in any case (fixed disable scrolling).

Here is how it looks for the index page

Screenshot 2021-09-01 at 18-27-28 Documentation · base v0 14 1 · OCaml Packages

Unfolded to the current module:

Screenshot 2021-09-01 at 18-27-02 Base Binary_searchable Make T · base v0 14 1 · OCaml Packages

for building a global navigation bar. Data comes from a newly generated
package.json file and some tests have been added for the parsing
Copy link
Collaborator

@tmattio tmattio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your work on this!

This LGTM overall - with some documentation for the added functions/modules, this is good to go I think!

lib/ocamlorg/module_map.ml Outdated Show resolved Hide resolved
lib/ocamlorg/module_map.ml Outdated Show resolved Hide resolved
lib/ocamlorg/module_map.mli Outdated Show resolved Hide resolved
lib/ocamlorg/module_map.mli Show resolved Hide resolved
@TheLortex
Copy link
Contributor Author

@tmattio I've added a commit with the interface change. Personally I prefer the original version because the usage is more concise: the interface change requires to transform a lot of Module_map.xxx into Module_map.Module.Map.xxx.

@tmattio
Copy link
Collaborator

tmattio commented Sep 10, 2021

Personally I prefer the original version because the usage is more concise: the interface change requires to transform a lot of Module_map.xxx into Module_map.Module.Map.xxx

I was expecting the toplevel module to become Module, so calls to Module_map.xxx would become Module.Map.xxx. But if the approach doesn't work well in practice, feel free to revert back to Module_map, I meant it as a suggestion, it's not blocking in any way 🙂

@TheLortex
Copy link
Contributor Author

Ah yes okay I finally get it. I think the Module name is too generic and might refer to other things in the future.
Here Module.t refers to the information we obtain from the module map (package.json). We could imagine other kinds Module.t (for example data extracted from an odoc file) so it makes sense to wrap it in the context of Module_map.

@tmattio
Copy link
Collaborator

tmattio commented Sep 10, 2021

LGTM, thanks @TheLortex, this is going to improve the navigation of packages documentation a lot!

@tmattio tmattio merged commit c984e79 into ocaml:main Sep 10, 2021
tmattio added a commit that referenced this pull request Sep 14, 2021
Co-authored-by: Thibaut <thibaut.mattio@gmail.com>
tmattio pushed a commit that referenced this pull request Sep 14, 2021
@maiste maiste mentioned this pull request Sep 14, 2021
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants