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

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

Closed
2 tasks done
brianrourkeboll opened this issue May 23, 2024 · 12 comments
Closed
2 tasks done

Comments

@brianrourkeboll
Copy link
Member

brianrourkeboll commented May 23, 2024

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

  • There were some kind of "expand/collapse all" button.

    Yes, I can click expand for every member individually if I want to browse examples, or I can throw

    for (const deets of document.getElementsByTagName('details')) {
        deets.setAttribute('open', 'true');
    }

    in the console, but it would be nice to have a button.

  • The <details>…</details> for a given member were auto-expanded when linked to directly.

    For example, linking directly to List.tryPick like https://fsharp.github.io/fsharp-core-docs/reference/fsharp-collections-listmodule.html#tryPick currently shows this:

    image

    but I think it would make sense to auto-expand the details for the linked member:

    image

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

@nojaf
Copy link
Collaborator

nojaf commented May 23, 2024

Sounds reasonable, go for it!

@nojaf
Copy link
Collaborator

nojaf commented May 31, 2024

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
Copy link
Member Author

@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
Copy link
Collaborator

nojaf commented May 31, 2024

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
Copy link
Member Author

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

let smry =
div [ Class "fsdocs-summary" ] [
yield! copyXmlSigIconForSymbolMarkdown m.Symbol
yield! copyXmlSigIconForSymbol m.Symbol
yield! sourceLink m.SourceLocation
fsdocsSummary m.Comment.Summary
]

@brianrourkeboll
Copy link
Member Author

brianrourkeboll commented May 31, 2024

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:

td [ Class "fsdocs-member-xmldoc" ] [
if List.isEmpty dtls then
smry
elif String.IsNullOrWhiteSpace(m.Comment.Summary.HtmlText) then
yield! dtls
else
details [] ((summary [] [ smry ]) :: dtls)

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
Copy link
Member Author

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.

expand-collapse.mp4

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
Copy link
Collaborator

nojaf commented Jun 4, 2024

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

@nhirschey
Copy link
Collaborator

nhirschey commented Jun 4, 2024

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
Copy link
Member Author

brianrourkeboll commented Jun 4, 2024

@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?

expand-collapse-2.mp4

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

@nhirschey
Copy link
Collaborator

Something like this?

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

@brianrourkeboll
Copy link
Member Author

Implemented in #917, #919, #920.

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

No branches or pull requests

3 participants