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

Added headings option to control optional headings, changed all headings to h1, #2729 #2733

Merged
merged 2 commits into from
Oct 9, 2024

Conversation

mrfigg
Copy link
Contributor

@mrfigg mrfigg commented Oct 7, 2024

Resolves #2729.

return (
<div class="tsd-page-title">
{!!props.model.parent && <ul class="tsd-breadcrumb">{context.breadcrumb(props.model)}</ul>}
{!props.model.isDocument() && (
<HeadingLevel class={classNames({ deprecated: props.model.isDeprecated() })}>
{(!props.model.isProject() || opts.project) && (!props.model.isDocument() || opts.document) && (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmmm.... I've realized that this is actually kind of problematic due to the readme. Both the index and the root modules pages are rendered with the project as the model.... and it's probably somewhat confusing that changing the project option will affect both of the pages...

Not quite sure how we ought to handle this yet... going to sleep on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I agree, not sure how that should be handled. We could add a headings.readme flag and then in header.tsx use something like the following:

//...
{
    (
        !props.model.isProject() ||
        (props.url === "index.html" && opts.readme) ||
        (props.url !== "index.html" && opts.project)
    ) &&
    (!props.model.isDocument() || opts.document) &&
    (<h1...> ... </h1>)
}
// ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

On second thought... the project page is really just another module, so this check should probably just be for index.html, I think we should always include the title on the modules page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, changed it to (props.url !== "index.html" || opts.readme).

@Gerrit0 Gerrit0 merged commit 98a7795 into TypeStrong:master Oct 9, 2024
5 checks passed
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Oct 9, 2024

Thanks!

@mrfigg mrfigg deleted the headings branch October 9, 2024 01:57
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

Successfully merging this pull request may close these issues.

Better page heading behavior
2 participants