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

WCAG upgrade - Assessment of Equal height plugin #9561

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hongbinyu413
Copy link

(Pick the language of your choice to fill out the following information. / Choisissez la langue de votre choix pour remplir l'information ci-dessous.)

What does this pull request (PR) do? / Que fait cette demande « pull » (PR)?

A clear and concise description of what the code of this PR accomplishes. /// Une description claire et concise de ce que le code de cette PR accompli.

Additional information (optional) / Information additionnelle (optionel)

General checklist / Liste de contrôle générale
Make your own list for the purpose of your Pull request. /// Faites votre propre liste en fonction des besoins de votre demande « pull ».

  • Create/update documentation /// Créer/mettre à jour la documentation
  • Build and test PR's code /// Rouler le script de compilation et tester le code de la PR
  • Validate changes against WCAG for accessibility /// Valider les changements avec WCAG pour l'accessibilité
  • Ensure documentation is bilingual /// S'assurer que la documentation soit bilingue

Related issues / Requêtes associées
List issues that are being closed or worked on with this pull request i.e. Closes #8941 /// Listez les autres requêtes (« issues » ou PR) qui sont fermées ou traitées avec cette demande de retrait ex. : Closes #8941

Screenshots / Captures d'écrans
If applicable, add screenshots to help demonstrate what this PR does. /// Si applicable, ajoutez des captures d'écran pour aider à démontrer ce que cette PR fait.

@hongbinyu413
Copy link
Author

@duboisp Could you take a look why the build errors?

@hongbinyu413 hongbinyu413 force-pushed the master branch 7 times, most recently from f52be77 to 58c706d Compare July 4, 2023 18:51
@duboisp duboisp reopened this Jul 5, 2023
@duboisp
Copy link
Member

duboisp commented Jul 10, 2023

@duboisp Could you take a look why the build errors?

I am looking at it right now.

Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Change required:

  • Put your name as the conformance report reviewer
  • Rename the few files that contains typo
  • Re-code as proposed the link to the report from the documentation page
  • Add and check conformance for the conformance requirement missing in the ACR report.
  • Review the date that was set on the assessment and on the conformance report
  • Fix the every assessment link in the conformance report.
  • Fix other typo as reported
  • Remove the test subject part (dct:hasPart) because I don't it is relevant here.
  • Fix the test subject info for the "equalheight/reports/1-accessment-wcag21.json"
  • Fix the assertor info for the "equalheight/reports/1-accessment-wcag21.json"

And about the continuous integration reported error, it is mostly about the broken link from the documentation page toward the report web page which are caused by a typo in the filename of the report. "cc" need to be replaced by "ss" in the word "assessment".

src/plugins/eqht-css/reports/1-accessment-wcag21-fr.hbs Outdated Show resolved Hide resolved
src/plugins/eqht-css/reports/1-accessment-wcag21-en.hbs Outdated Show resolved Hide resolved
src/plugins/equalheight/reports/1-accessment-wcag21-en.hbs Outdated Show resolved Hide resolved
src/plugins/equalheight/reports/1-accessment-wcag21-fr.hbs Outdated Show resolved Hide resolved
src/plugins/eqht-css/reports/1-accessment-wcag21.json Outdated Show resolved Hide resolved
src/plugins/equalheight/reports/1-acr-wcag21.json Outdated Show resolved Hide resolved
src/plugins/equalheight/reports/1-acr-wcag21.json Outdated Show resolved Hide resolved
src/plugins/equalheight/reports/1-acr-wcag21.json Outdated Show resolved Hide resolved
src/plugins/equalheight/reports/1-accessment-wcag21.json Outdated Show resolved Hide resolved
src/plugins/eqht-css/reports/1-acr-wcag21.json Outdated Show resolved Hide resolved
@hongbinyu413 hongbinyu413 force-pushed the master branch 7 times, most recently from c1f3367 to 08c5bf3 Compare July 11, 2023 15:14
@hongbinyu413 hongbinyu413 force-pushed the master branch 2 times, most recently from b53a128 to 0435bbd Compare July 11, 2023 15:50
site/pages/docs/ref/eqht-css/eqht-css-en.hbs Outdated Show resolved Hide resolved
site/pages/docs/ref/eqht-css/eqht-css-fr.hbs Outdated Show resolved Hide resolved
src/plugins/equalheight/reports/1-assessment-wcag21.json Outdated Show resolved Hide resolved
src/plugins/eqht-css/reports/1-accessment-wcag21.json Outdated Show resolved Hide resolved
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

This is a partial review, some of the previous change request that are marked "Outdated" still need to be addressed in the renamed file.

I noted the change required to fix the broken link reported by the Continuous Integration script.

site/pages/docs/ref/eqht-css/eqht-css-fr.hbs Outdated Show resolved Hide resolved
site/pages/docs/ref/eqht-css/eqht-css-en.hbs Outdated Show resolved Hide resolved
src/plugins/eqht-css/reports/1-acr-wcag21.json Outdated Show resolved Hide resolved
src/plugins/equalheight/reports/1-assessment-wcag21.json Outdated Show resolved Hide resolved
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

There is still a few change required but my local testing was creating unexpected issue like the diff illustrated in this PR don't match the file I do have locally. For example, locally the JSON is broken but I can see it complete here in the diff.

So I do have the impression the "merge" commit do create a code regression situation. Please remove the merge commit and revise the code to ensure that everything is fixed as expected.

Thanks

src/plugins/equalheight/reports/1-assessment-wcag21.json Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Query: Project item Support a github project
Projects
Status: For Review
Development

Successfully merging this pull request may close these issues.

Question about colour and palettes
2 participants