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

Use shiki-twoslash for docs #3324

Closed
Rich-Harris opened this issue Jan 13, 2022 · 5 comments
Closed

Use shiki-twoslash for docs #3324

Rich-Harris opened this issue Jan 13, 2022 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@Rich-Harris
Copy link
Member

Describe the problem

These are the endpoint docs rendered on my screen:

image

There's a full page of TypeScript throat-clearing before we get to the bit that people actually want to see:

image

It's useful to have the types documented, but not like this.

Another problem with our docs: the types can get out of date, as seen here. It would be nice if somehow snippets in our docs were typechecked.

Describe the proposed solution

We could potentially solve both problems with @orta's https://shikijs.github.io/twoslash.

Questions to which I don't know the answer:

  • Can it be made to work with Svelte components?
  • Does it work with JSDoc syntax?
  • Do we have any concerns about accessibility, if type information is hidden behind hovers?

Aside: doing this would presumably require cooperation between this repo, sveltejs/sites, and sveltejs/action-deploy-docs. Before leaping into it we might want to consider whether the current setup is to our liking, or whether we might want to move to multi-page docs with search and interactive components (likely powered by mdsvex).

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

@dummdidumm dummdidumm added the documentation Improvements or additions to documentation label Jan 13, 2022
@geoffrich
Copy link
Member

re: a11y - the type annotations shiki-twoslash shows on hover are not accessible, by keyboard or by screen reader. Right now it just hide/shows a pseudoelement on :hover. I'm honestly not sure if there's a good way to make it accessible, especially since tooltips are infamously tricky in that regard.

Though I definitely see the value it brings, especially around keeping the docs from getting out of date. I don't think the type annotations not being accessible is a dealbreaker if we make sure the type info is available somewhere else (details/summary block? separate page? not sure). I think having the type info available outside of a hover is good anyway, since people might want to reference them without needing to keep hovering over a section of the docs.

@jasonlyu123
Copy link
Member

It seems like we could use our CSS to provide the hover, focus style. So that we can make the tooltip also show up with focus. The one on the website seems to be from the CSS in their site instead of the library.

Can it be made to work with Svelte components?

I did some simple POC. And make it somewhat work with the svelte file with something similar to what we did in the svelte-language server. By running shiki-twoslash against the code transformed by svelte2tsx. And sourcemap the position back to the original code. But because it only touches the generated code. It means we have to write our logic to remove the special directive comments, or simply don't use them.

Does it work with JSDoc syntax?

Yes. We just have to tell the library the file is javascript instead of typescript.

@orta
Copy link

orta commented Jan 30, 2022

My first implementation of the hovers used JavaScript to move around a singleton popup, which could definitely trigger the right screen-reader calls when you need them. All the key info is kept in the DOM 👍🏻

I talked with the a11y folks at Microsoft, and we kinda chatted that the // ^? feature was largely good enough to cover the cases where that highlight information was knowingly useful because it was then inlined into the code and readable like everything else. (vs the rest of the hover info which is incidentally useful)

Open to ideas though, and @jasonlyu123 - yeah, I could see that working - the API primitives in shiki-twoslash should be enough

@Rich-Harris Rich-Harris mentioned this issue Feb 3, 2022
7 tasks
@geoffrich
Copy link
Member

Yeah, I was mainly referring to the hover info not being exposed. We could potentially expose it by setting a focus style, but then we'd have to give each hoverable element a tabindex so it could be focused, and an accessible name so it was properly identified to screen readers... personally, I think that would cause more trouble than it's worth. If we tried to do that, people navigating by keyboard would be annoyed by the number of tab stops on the page, since every variable in a code block would now receive focus when tabbing. It's less intrusive to keep it a hover style.

(vs the rest of the hover info which is incidentally useful)

I think this is the main thing I wanted to confirm -- as long as the type info in the hovers is available elsewhere, I think we're good from an accessibility standpoint. I just don't think it can completely replace the existing type definitions if people still need that info (not sure if that was being proposed or not).

@Rich-Harris
Copy link
Member Author

Since #3986 is merged, I'll close this. I opened sveltejs/svelte.dev#649 to track adding twoslash support to .svelte blocks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

5 participants