KEMBAR78
Add index to core spec by mattgarrish · Pull Request #2288 · w3c/epub-specs · GitHub
Skip to content

Conversation

mattgarrish
Copy link
Member

@mattgarrish mattgarrish commented May 13, 2022

Adds the respec-generated index of terms. I don't think this is useful for any of the other specs as they don't add similar numbers of new terms.

Fixes #2260


💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on May 17, 2022, 11:44 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Spec Generator - Spec Generator is the web service used to build specs that rely on ReSpec.

🔗 Related URL


😭  Sorry, there was an error generating the HTML. Please report this issue!
Specification: http://labs.w3.org/spec-generator/uploads/KfgrHj?isPreview=true&publishDate=2022-05-17
ReSpec version: 32.1.6
File a bug: https://github.com/w3c/respec/
Error: Error: Evaluation failed: Timeout: document.respec.ready didn't resolve in 27763ms.
    at ExecutionContext._evaluateInternal (/u/spec-generator/node_modules/puppeteer/lib/cjs/puppeteer/common/ExecutionContext.js:221:19)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async ExecutionContext.evaluate (/u/spec-generator/node_modules/puppeteer/lib/cjs/puppeteer/common/ExecutionContext.js:110:16)
    at async generateHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:221:12)
    at async toHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:92:18)
    at async Object.generate [as respec] (file:///u/spec-generator/generators/respec.js:15:44)
    at async file:///u/spec-generator/server.js:244:48

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@mattgarrish mattgarrish requested review from dauwhe and iherman May 13, 2022 16:08
@mattgarrish
Copy link
Member Author

Copy link
Member

@iherman iherman left a comment

Choose a reason for hiding this comment

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

Overall, I think it is worth adding it, even if not perfect.

One tiny thing. The generated code seems to be:

<a class="index-term" href="....">
    "container resource"
</a>

for elements and then

<a class="index-term" href="....">
    <code>meta</code>
</a>

for elements. Would it be better if we added to our css something like

a.index-term code {
   color: #c63501 !important;
}

we use that colour all over the place for code, better keep it in the index, too...

@mattgarrish
Copy link
Member Author

Would it be better if we added to our css

Should this be raised against respec?

It's respec that colours the code tags in the body, so if we manually override the index then we have to keep watch that the respec css doesn't change in the future.

@iherman
Copy link
Member

iherman commented May 15, 2022

Would it be better if we added to our css

Should this be raised against respec?

It's respec that colours the code tags in the body, so if we manually override the index then we have to keep watch that the respec css doesn't change in the future.

Oops, somehow I always thought that we set the color, not respec...

Yes, that should be a respec issue. Not only for this case; I have added the following entries to our common css files to ensure the right colour when using xref:

code a[data-type~="element"], 
code a[data-type~="dfn"],
code a[data-type~="element-attr"]
{
    color: #c63501 !important;
}

These should be superfluous as well.

Do we want to start by setting it for ourselves and raise an issue afterwards, or just raise an issue? I would be in favour of the former; I do not know whether such an issue would be taken care of with high priority...

@mattgarrish
Copy link
Member Author

Do we want to start by setting it for ourselves and raise an issue afterwards

That works for me. I'll update the PR when I have a minute.

@iherman
Copy link
Member

iherman commented May 15, 2022

Also: do we want to switch indexing on for RS? It does not define many new concepts, but I also found the generated list of external concept references useful... (I cannot judge the value for the A11y document).

@mattgarrish
Copy link
Member Author

mattgarrish commented May 16, 2022

Do we want to start by setting it for ourselves and raise an issue afterwards

That works for me.

Although, looking at the result of adding custom CSS, I'm not so sure it's a good idea. Linked code is now hard to distinguish from unlinked code as there may only be a faint dotted underline. We also get links that are part orangey-red and part blue if they include a code-tagged word with other words. It's really quite messy. It might be better to remove the custom CSS and figure out with the respec/spec-prod folks what should be done.

@mattgarrish
Copy link
Member Author

It does not define many new concepts, but I also found the generated list of external concept references useful...

I'm of the opposite view: it's rather clunky and duplicative. It appears to take the text of every a element and add an entry. If we link to definitions and sections using the same words, we get different entries, although nothing makes clear which is which until you follow the link into the body and then click on that link to see where it goes.

Here's a random sampling I spotted in the RS spec only looking at a few "p" entries:

- package document
- package document
...
- properties
- properties attribute
- properties attribute
...
- publication resources
- publication resources

@mattgarrish
Copy link
Member Author

I'm opting for a middle road on this and adding indexes to the three rec-track specs but removing the CSS link overrides so we have consistent colouring of our links.

@mattgarrish mattgarrish merged commit 953a0ea into main May 17, 2022
@mattgarrish mattgarrish deleted the feature/add-index branch May 17, 2022 11:55
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.

Index for spec?

2 participants