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

Documentation: Improve the fields section of the Manifest reference document #308

Closed
seankmartin opened this issue Aug 9, 2023 · 16 comments · Fixed by #326
Closed

Documentation: Improve the fields section of the Manifest reference document #308

seankmartin opened this issue Aug 9, 2023 · 16 comments · Fixed by #326

Comments

@seankmartin
Copy link

The manifest reference fields section isn't displaying correctly at https://napari.org/dev/plugins/technical_references/manifest.html#fields for two reasons:

  1. The part in icon should have the three list items below it as part of that table entry. I guess the indentation is off, because the table does not display properly.
  2. Above the table it reads; "All fields are optional except those in bold.", but none of the fields are in bold by the looks. I don't know if it is possible to make a bold code block, and if not, maybe this sentence could be removed.
@nclack
Copy link
Collaborator

nclack commented Aug 9, 2023

thanks for the report! Looking at this.

Something is definitely screwing up how the table gets generated.

It looks like the fields are bolded twice: once by the <strong> and once by the way <code> is styled. Maybe there's another way we can indicate which fields are required.

@nclack
Copy link
Collaborator

nclack commented Aug 9, 2023

The icon description text (here) isn't working with the way the table is generated here.

In particular, field descriptions with newlines look incompatible.

@lucyleeow
Copy link
Contributor

image

For reference, this is what it looks like.

@lucyleeow
Copy link
Contributor

Just checking, @melissawm are you looking into this?

Also out of interest, would you be able to view the built docs in npe2 CI's? Would that be "docs render"?

@nclack
Copy link
Collaborator

nclack commented Aug 30, 2023

would you be able to view the built docs in npe2

Yes, in theory.

Start with python _docs/render.py (like in the ci). That puts some files in the 'docs' folder.

Then you should be able to render the docs folder with jupyterbook (jb build ./docs)

I'm forgetting a step though (or it's broken). The toc doesn't seem to be picking things up properly.

@lucyleeow
Copy link
Contributor

Thanks, just to confirm, you can only do this locally not in the CI ?

@melissawm
Copy link
Member

I am, but here's my issue so far:

Markdown doesn't accept a list syntax inside a table. The alternative would be to use html tags such as

<ul>
 <li> List item 1</li>
 <li> List item 2</li>
</ul>

This works, BUT: the manifest reference document is auto-generated from https://github.com/napari/npe2/blob/main/_docs/render.py, and from the quick investigation I did this creates documentation based on https://github.com/napari/npe2/releases/latest/download/schema.json, not the local version of the file. So the fix would be to replace the nested lists inside the table in the remote schema.json file, and that should fix the rendering.

@lucyleeow
Copy link
Contributor

Ah that sounds complicated!

@melissawm
Copy link
Member

I'm happy to send a pr to the schema.json file but I just want to confirm with one of the maintainers that this is the right approach - it is kinda tricky 😅

@DragaDoncila
Copy link
Contributor

@melissawm I am super keen to get this fixed asap (even if we end up changing or simplifying docs rendering somehow down the track, which I'm very open to).

So the fix would be to replace the nested lists inside the table in the remote schema.json

Is this something you can do, or do we need idk permissions or security tokens or something? If it's something you can do, please go ahead and do it and let me know what's needed from our end and I'll make sure we get it done.

Agree that the rendering from the remote schema is a little unwieldy - I assume it's being done to make sure the docs match the released version, but this is a little hazy with the versioned napari docs - what version of these docs is actually displayed on napari.org? The latest released npe2 version? Whatever version is there when the released docs get built?

@lucyleeow
Copy link
Contributor

lucyleeow commented Sep 20, 2023

I assume it's being done to make sure the docs match the released version, but this is a little hazy with the versioned napari docs - what version of these docs is actually displayed on napari.org? The latest released npe2 version? Whatever version is there when the released docs get built?

For info, when I made a change in #310, I note that this change is not reflected in dev: https://napari.org/dev/plugins/building_a_plugin/guides.html#widgets (though dev docs are different from stable). Are npe2 releases co-ordinated with napari releases? if not this makes 'dev' and 'stable' a bit tricky.

@psobolewskiPhD
Copy link
Member

Looking at
https://github.com/napari/docs/blob/6e280b08d69dd18a9a5eff3ffce7f86e0ec12c3e/docs/_scripts/prep_docs.py#L15-L28

Looks like we npe2 version will always be latest released version.
I think ideally we would want dev docs to clone the npe2 repo and released docs (stable) to use released npe2?
Or honestly thinking further, since npe2 docs audience is developers, maybe npe2 docs are just dev docs and we should always clone the repo instead of using a release?

@nclack
Copy link
Collaborator

nclack commented Sep 20, 2023

I think ideally we would want dev docs to clone the npe2 repo and released docs (stable) to use released npe2?

sounds like a good idea. This might be a good issue for napari/docs?

@psobolewskiPhD
Copy link
Member

I think it needs to be a fix in both the docs and napari repo workflows.

@jni
Copy link
Member

jni commented Oct 11, 2023

I think it needs to be a fix in both the docs and napari repo workflows.

Just gonna leave this here... napari/napari#6049 😂

@melissawm could you summarise what the build workflow for this table is? I am lost from reading the discussion above. It sounds like:

  • read schema.json
  • autogenerate a markdown table based on that
  • embed the table in the docs
  • build the docs

Is that right? Because if so then I guess it doesn't hurt to use the HTML table syntax instead of markdown?

@melissawm
Copy link
Member

Long story short: the PR above fixes this, but then a new schema.json file needs to be generated and uploaded to https://github.com/napari/npe2/releases/latest/download/schema.json

nclack added a commit that referenced this issue Nov 24, 2023
The schema.json file at
https://github.com/napari/npe2/releases/latest/download/schema.json also
needs to be updated after this PR is merged.

Fixes #308 (not the bold part though, that will only be fixed with the
new theme)

Co-authored-by: Nathan Clack <nclack@chanzuckerberg.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants