fsprojects / FSharp.Formatting

F# tools for generating documentation (Markdown processor and F# code formatter)
https://fsprojects.github.io/FSharp.Formatting/
Other
462 stars 155 forks source link

Suggestion: auto-expansion of `<details>…</details>` for direct links #916

Closed brianrourkeboll closed 3 weeks ago

brianrourkeboll commented 1 month ago

While #778, as implemented in #818, does indeed make for easier skimming of API docs, I think it would be nice if:

(I'd be willing to open a PR for these.)

nojaf commented 1 month ago

Sounds reasonable, go for it!

nojaf commented 1 month ago

Hmm, still some remaining problems with the new approach:

https://fsharp.github.io/fsharp-compiler-docs/reference/fsharp-compiler-codeanalysis-fsharpchecker.html#InvalidateAll

image

brianrourkeboll commented 1 month ago

@nojaf Hmm, I'll look.

Separately, I'm noticing else on that same page that will necessitate some additional tweaks in that area anyway.

The .fsdocs-entity-xmldoc { > div { flex-direction: row reverse } }, which was already there, affects the order of any paragraphs defined with <para> that are nested in the <summary>.

So these paragraphs

/// <summary>
/// <para>
///   Parse and check a source code file, returning a handle to the results
/// </para>
/// <para>
///    Note: all files except the one being checked are read from the FileSystem API
/// </para>
/// <para>
///   Return FSharpCheckFileAnswer.Aborted if a parse tree was not available.
/// </para>
/// </summary>

— are shown in reverse order as

image

I think they should at least be shown in the order in which they're written, and it would probably be better if they were separated vertically instead of horizontally.

nojaf commented 1 month ago

Hmm, yeah, you're right. That row reverse was probably there for a reason. I'm okay with changing it, but we might need to consider any other issues it could introduce.

brianrourkeboll commented 1 month ago

Yeah, the contents themselves are generated in reverse order, only to be reversed again by the CSS, although I don't understand why:

https://github.com/fsprojects/FSharp.Formatting/blob/56b856bc3be5ab7ff7c1bbcb30fe0f3e758235cc/src/FSharp.Formatting.ApiDocs/GenerateHtml.fs#L183-L189

brianrourkeboll commented 1 month ago

Hmm, still some remaining problems with the new approach:

https://fsharp.github.io/fsharp-compiler-docs/reference/fsharp-compiler-codeanalysis-fsharpchecker.html#InvalidateAll

image

Here's the explanation for this particular thing:

image

Sometimes the <div class="fsdocs-summary"> will be nested inside <details><summary>, and sometimes it won't be:

https://github.com/fsprojects/FSharp.Formatting/blob/56b856bc3be5ab7ff7c1bbcb30fe0f3e758235cc/src/FSharp.Formatting.ApiDocs/GenerateHtml.fs#L271-L277

I removed the CSS for the bare div case in my PR, because I didn't understand that there were these different cases.

That conditional actually also means that, if there is no <summary> in the XML doc, there will be no source links, even if there are other elements:

image

I don't think there's a good reason not to show source links in such a case.

brianrourkeboll commented 3 weeks ago

If I did add an expand/collapse-all button, does this look like a reasonable place to put it? The idea would be for it to save your preference for the whole site to local storage, like the theme toggle.

https://github.com/fsprojects/FSharp.Formatting/assets/14795984/cef2961a-6619-445c-8dfe-2de4ec56dcde

I guess it could also only be shown on pages that had API docs, but then would it still make sense to show it in the top right? Or somewhere else?

nojaf commented 3 weeks ago

My 2 cents: it would only make sense on pages with API docs and should not be part of the menu bar.

nhirschey commented 3 weeks ago

If it’s easy, to me it seems more intuitive on the same row as “instance members”. For me it doesn’t seem obvious to look for this on the menu bar. I expect expand/collapse to be located closer to the elements it affects.

(But if this does not appeal to you ignore it).

brianrourkeboll commented 3 weeks ago

@nhirschey

If it’s easy, to me it seems more intuitive on the same row as “instance members”. For me it doesn’t seem obvious to look for this on the menu bar. I expect expand/collapse to be located closer to the elements it affects.

Something like this?

https://github.com/fsprojects/FSharp.Formatting/assets/14795984/040326cb-ca9e-4908-871a-5e0f9eeb9aed

(Could also be with "Instance members" as you suggest instead of in the table header.)

nhirschey commented 3 weeks ago

Something like this?

Yeah, exactly. To me that’s more intuitive.

brianrourkeboll commented 3 weeks ago

Implemented in #917, #919, #920.