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

More LabIcon docs #8246

Merged
merged 14 commits into from
Apr 29, 2020
Merged

Conversation

telamonian
Copy link
Member

References

This PR adds a background section, and some sections on creating custom LabIcon to the LabIcon docs in the @jupyterlab/ui-components README.

Since these are narrative docs, and since the jlab API docs are somewhat invisible, these LabIcon docs should probably also go somewhere in the main jlab docs. Where should I put them?

Code changes

NA

User-facing changes

NA

Backwards-incompatible changes

NA

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

@telamonian telamonian changed the title More Labicon docs More LabIcon docs Apr 22, 2020
@jtpio
Copy link
Member

jtpio commented Apr 22, 2020

Thanks!

Since these are narrative docs, and since the jlab API docs are somewhat invisible, these LabIcon docs should probably also go somewhere in the main jlab docs. Where should I put them?

At the moment there seems to be two options:

@telamonian
Copy link
Member Author

Oh. I guess I wrote https://jupyterlab.readthedocs.io/en/stable/developer/extension_points.html#icons at some point and forgot about it

@telamonian
Copy link
Member Author

In b921254 I came up with a way to share the same labicon.md source file between the API docs and the main jlab docs that I'm reasonably happy with.

Problem is, now the API docs are a bit broken: the markdown link to labicon.md that I added to ui-components/README.md only works locally, it doesn't work in the rendered API docs.

Gotta think of something else

@telamonian
Copy link
Member Author

telamonian commented Apr 23, 2020

Alright, I think that's a wrap for this PR. Here's the user facing changes (all to docs):

  • developer docs

    • added several sections to the icons docs
      • a couple of background sections
      • a couple of sections about creating your own LabIcon instances
      • a section with complete information about how to sync icon colors to the current theme
    • I broke out the LabIcon docs, along with any future ui-component docs, out into their own "Reusing JupyterLab UI" section
    • The previously existing "Icons" section in "Common Extension Points" is now just a link to the above
  • API docs

    • as per the dev docs, added the same set of new sections to the LabIcon docs

In terms of non-visible changes, I didn't want to have to keep manually syncing up two nearly identical LabIcon sections in the dev and API docs, so I came up with various ways to generate both from a single source. This turned out to be fairly tricky (one is markdown, the other is rst, they both needed different document titles, etc). I don't think I've yet found the perfect way of doing this, but what I currently have is good enough for this PR.

Briefly, there's now a docs:init command in @jupyterlab/ui-components. Calling this will rebuild both packages/ui-components/README.md and docs/source/developer/ui_components.rst from a set of document sources that can be found at packages/ui-components/docs

@telamonian
Copy link
Member Author

One possibly much simpler alternative would be to link to the API docs from the dev docs, or vice versa. I'll look into this and other possibilities while I work on making the API docs nicer overall over at #8239

svgstr: fooSvgstr
});

Sync icon color to JupyterLab theme
Copy link
Member

Choose a reason for hiding this comment

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

I am not too keen about having duplicate text in three places checked into VCS, because it seems likely we might update it in one place and forget the other two. Any ideas on how to make this have a single source of truth?

Copy link
Member Author

Choose a reason for hiding this comment

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

Any ideas on how to make this have a single source of truth?

Nope, but I'll take suggestions :)

More seriously, I do have some notions about how to improve the docs:init build here, but they're all waaay out of scope for what's supposed to be a PR updating the LabIcon docs. I am actively working on improving the API docs, and may be able to fold this issue into that work as well.

For now, we treat the sources in ui-components/docs as the source of truth. Running jlpm docs:init (while in ui-components) will automatically propagate any changes to the README and the dev docs. If anyone does end up commiting changes directly to the wrong copy, I'll deal with manually resolving the conflicts

Copy link
Member Author

Choose a reason for hiding this comment

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

I've now added a warning comment to the top of the autogenerated files:

<!--
THIS FILE IS AUTOGENERATED, DO NOT EDIT

Instead, make changes to docs sources in `packages/ui-components/docs`,
then run "jlpm docs:init" to refresh the built docs
-->

Copy link
Member

Choose a reason for hiding this comment

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

Running jlpm docs:init (while in ui-components) will automatically propagate any changes to the README and the dev docs. If anyone does end up commiting changes directly to the wrong copy, I'll deal with manually resolving the conflicts

For the README maybe we could just link to that file instead of copying? And maybe for the docs we could have it do that live generation when generating the docs instead of storing it in VCS?

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

basically, I don't want to roll something project wide and complicated that I'm just going to remove in a future PR. Keeping the docs checked in in 3 places is dumb, but it's also simple and renders reliably

Copy link
Member

@saulshanabrook saulshanabrook left a comment

Choose a reason for hiding this comment

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

Thank you so much max! This will be really helpful, both for maintaining the icon features and for people who want to use icons or add new ones.

Just had a few comments.

@telamonian telamonian added this to the 2.2 milestone Apr 28, 2020
@telamonian
Copy link
Member Author

I want the improved LabIcon docs to be available to people ASAP, so I'm going to pull this in now and save the improvements to how those docs are built for a later PR

@telamonian telamonian merged commit 6d106df into jupyterlab:master Apr 29, 2020
@blink1073
Copy link
Member

@meeseeksdev please backport to 2.1.x

@lumberbot-app
Copy link

lumberbot-app bot commented Apr 30, 2020

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout 2.1.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 6d106df4276e39b7726083b9d1bd3166b8a5c74b
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #8246: More LabIcon docs'
  1. Push to a named branch :
git push YOURFORK 2.1.x:auto-backport-of-pr-8246-on-2.1.x
  1. Create a PR against branch 2.1.x, I would have named this PR:

"Backport PR #8246 on branch 2.1.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 10, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation pkg:ui-components status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants