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

Add search role’s base concept pointing to HTML spec. Fix #1898 #1900

Merged
merged 2 commits into from
Apr 24, 2023

Conversation

thibaudcolas
Copy link
Contributor

@thibaudcolas thibaudcolas commented Mar 27, 2023

Claiming #1898 – with a link directly to the HTML spec.

Couple of things I noted along the way that informed this task, however small:

  • The reference style I chose here ("direct link to the correct element in the spec") is only used for 3 other base concepts. The rest use the style: <code>&lt;button&gt;</code> in [[HTML]], with "HTML" being an inline reference pointing to the doc’s Normative references section. I found the direct links more helpful so chose that.
  • This uses the same link (https://www.w3.org/TR/html5/grouping-content.html) to the HTML spec as other Base Concept links for consistency, but this URL 307-redirects to the WHATWG living standard athttps://html.spec.whatwg.org/multipage/grouping-content.html. It’d be nice to avoid this redirect but felt consistency mattered more, though the HTML link in "Normative references" also points to the WHATWG multi-page spec.

I’ll happily revisit this if it’s better to use the no-direct-link reference for consistency. And clean up other definitions as a separate PR if it helps.


Preview | Diff

Copy link
Member

@adampage adampage left a comment

Choose a reason for hiding this comment

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

LGTM. 👍🏻 I find the direct link helpful, but I also don’t know why there are differing conventions in the spec. Maybe it’d be good to normalize those, as a separate issue? /cc @spectranaut

@spectranaut
Copy link
Contributor

LGTM. 👍🏻 I find the direct link helpful, but I also don’t know why there are differing conventions in the spec. Maybe it’d be good to normalize those, as a separate issue? /cc @spectranaut

I honest don't think I'm the best person to ask -- but maybe our links do need auditing/normalizing/fixing, maybe an issue could be filed? :)

I think an editor, @pkra or @jnurthen should way in on the linking decision here :)

index.html Outdated Show resolved Hide resolved
@jnurthen
Copy link
Member

I’ll happily revisit this if it’s better to use the no-direct-link reference for consistency. And clean up other definitions as a separate PR if it helps.

A PR to clean up the others to use either xref (if possible) or data-cite (if no reasonable xref exists) would be great!

@w3cbot
Copy link

w3cbot commented Mar 31, 2023

jnurthen marked as non substantive for IPR from ash-nazg.

Copy link
Member

@scottaohara scottaohara left a comment

Choose a reason for hiding this comment

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

i suggest we go with @jnurthen's second suggestion of using xref - [^search^]

lgtm otherwise

Co-authored-by: James Nurthen <jnurthen@users.noreply.github.com>
@thibaudcolas
Copy link
Contributor Author

👍 thank you all for the feedback, this is my first time using ReSpec so very much appreciated. I’ve updated the PR accordingly.

@jnurthen jnurthen dismissed scottaohara’s stale review April 5, 2023 16:20

change made in response

@jnurthen jnurthen merged commit c77c2c4 into w3c:main Apr 24, 2023
github-actions bot added a commit that referenced this pull request Apr 24, 2023
SHA: c77c2c4
Reason: push, by jnurthen

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@thibaudcolas thibaudcolas deleted the 1898-search-role-base-concept branch April 25, 2023 14:14
jnurthen added a commit that referenced this pull request Oct 10, 2023
Co-authored-by: James Nurthen <jnurthen@users.noreply.github.com>
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.

6 participants