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

[Monorepo] Netlify to replace pr-review #2242

Open
3 of 5 tasks
spectranaut opened this issue Jun 13, 2024 · 7 comments
Open
3 of 5 tasks

[Monorepo] Netlify to replace pr-review #2242

spectranaut opened this issue Jun 13, 2024 · 7 comments
Assignees
Labels
editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo

Comments

@spectranaut
Copy link
Contributor

spectranaut commented Jun 13, 2024

We also want to look to add the follow features to our netlify publications:

@spectranaut spectranaut added the editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo label Jun 13, 2024
@daniel-montalvo daniel-montalvo changed the title Netlify to replace pr-review [Monorepo] Netlify to replace pr-review Jul 8, 2024
@daniel-montalvo
Copy link
Contributor

daniel-montalvo commented Jul 8, 2024

We now have statically-generated PR previews
#2265
All pages changed can be viewed from within the "Checks" tab, for example in https://github.com/w3c/aria/pull/2284/checks/> Netlify > Page changed

@spectranaut
Copy link
Contributor Author

About Eventually, we will need successful links between specs generated by the PR instead of links to the editor's drafts -- If, for core-aam, we add the ED links like this:

  ariaSpecURLs: {
    ED: "../index.html",
    CR: "https://w3c.github.io/aria/",
    CRD: "https://w3c.github.io/aria/",
    PR: "https://www.w3.org/TR/wai-aria/",
    REC: "https://www.w3.org/TR/wai-aria/",
  },
  accNameURLs: {
    ED: "../accname/index.html",
    CR: "https://w3c.github.io/accname/",
    CRD: "https://w3c.github.io/accname/",
    PR: "https://www.w3.org/TR/accname-1.2/",
    REC: "https://www.w3.org/TR/accname-1.2/",
  },

It works in PR preview, which you could see before I realized w3c.github.io/core-aam editor draft would be broken: #2298

So maybe we really do need to set up all the w3c.github.io/* to directed back to this repository. Then if we do this for all the specs, the links will be correct in PR preview... right...?

@daniel-montalvo
Copy link
Contributor

@spectranaut @pkra @jnurthen and all.

I have followed an approach similar to @spectranaut comments above in https://github.com/daniel-montalvo/aria

The build process now has a bash script that replaces specStatus with "unofficial" and then includes the unofficial keys for the resolveReferences function to generate relative links in the PRs.

For a prototype of how this could work, see
https://staging-aria.netlify.app
and add / to get to the other specs as desired.

I am using the Netlify production context for now because this is just a test. If we are happy with this approach, I will adjust the main ARIA netlify configuration file accordingly so that this only runs in PRs.

@pkra
Copy link
Member

pkra commented Feb 4, 2025

@daniel-montalvo I thought #2325 and #2386 had fixed cross references to link within PRs but on #2416 in https://deploy-preview-2416--wai-aria.netlify.app I find ~5000 links starting with https://w3c.github.io/aria/#. Should I open a new issue?

@daniel-montalvo
Copy link
Contributor

Daniel to have a look at links that take to github.io

An example is list item.

@daniel-montalvo daniel-montalvo self-assigned this Feb 12, 2025
@pkra pkra moved this to In Progress in ARIA Editors Feb 12, 2025
@spectranaut
Copy link
Contributor Author

spectranaut commented Mar 6, 2025

Hmm @daniel-montalvo I'm trying to track down the fix for relative links in pr previews. Did you ever merge in the work you mentioned (in this comment above)[https://github.com//issues/2242#issuecomment-2271738905]?

There are a few PRs that reference this issue and mention relative links, but they both look like they were closed without merging and there is a zero diff in both of them: #2325 and #2386

Currently, the links that work are those that are generated by as relative links, for example, respec links to definitions.

The problem is that all the <rref> and <pref> and generated entries in the Characteristics table all go back to the editors draft, instead of being relative. Did we think we solved this, by the bash script you mentioned @daniel-montalvo? I think the script didn't make it into the main branch somehow, unless I'm missing something.

@daniel-montalvo
Copy link
Contributor

Thanks for pointing this out, Valerie.

Bummer... I thought this was merged and restored my fork discarding all of these. Will have to recreate this.

pkra pushed a commit that referenced this issue Mar 24, 2025
Partially addresses #2242

This only picks up references to roles, states, and properties. Some definitions are still pointing to the editors draft or TR documents because they are not processed by the script due to the way they are written.

For example, `<a>assistive technologies</a>` would rely on [Xref](https://respec.org/xref) for processing.
We should decide if we are happy with landing this or we want another approach, like waiting for respec to process the files and then just search and replace all absolute links (no matter where they link to) with relative links.


Co-authored-by: daniel-montalvo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo
Projects
Status: In Progress
Development

No branches or pull requests

3 participants