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

Only include endpoint path in <summary> #1446

Merged
merged 2 commits into from
Mar 2, 2023
Merged

Only include endpoint path in <summary> #1446

merged 2 commits into from
Mar 2, 2023

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Feb 22, 2023

As threatened on #1358 (comment) and #1352 (comment). (If nothing else, I seem to be consistent.)

Fixes #1352.

Preview: https://pr1446--matrix-spec-previews.netlify.app

@DMRobertson
Copy link
Contributor Author

DMRobertson commented Feb 22, 2023

Seems to fix #1352. I can't remember the details but I vaguely remember from the last time I looked into this that there's something about being inside a <summary> that changes how lists are laid out.

Edit: #1358 (comment)

@DMRobertson DMRobertson marked this pull request as ready for review February 22, 2023 21:20
@DMRobertson DMRobertson requested a review from a team as a code owner February 22, 2023 21:20
@DMRobertson

This comment was marked as outdated.

@DMRobertson
Copy link
Contributor Author

I do seem to have lost the borders

This seems to be local to me and was because I had uncommitted changes in themes/docsy.

@richvdh richvdh self-requested a review March 2, 2023 16:04
Comment on lines +296 to 300
h1 {
margin: 0;
/* Ensure the disclosure control is vertically centred with the header text. */
vertical-align: middle;
}
Copy link
Member

Choose a reason for hiding this comment

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

I guess this relies on the first line of the summary being an <h1>, but that's probably an assumption we can live with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could maybe do summary > * or summary > :first-child if this bites us in the future/

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

thanks!

@richvdh richvdh merged commit f9ea0f9 into matrix-org:main Mar 2, 2023
clokep pushed a commit to clokep/matrix-spec that referenced this pull request May 3, 2023
@DMRobertson
Copy link
Contributor Author

Other details elements have this wierdly oversized summary too, e.g https://spec.matrix.org/v1.7/client-server-api/#mroomcanonical_alias

richvdh pushed a commit that referenced this pull request May 30, 2023
I previously made this change for HTTP endpoints in #1446. It seems there are
other places where we have over-sized `<summary>` elements. I would like
to elimintate those too, as grumbled about in
#1358 (comment)
and
#1352 (comment)
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.

Borked styling of list items under /register definition
2 participants