Closed giacomo-petri closed 5 months ago
Just reviewed Carlos' feedback here https://github.com/act-rules/act-rules.github.io/issues/1999#issuecomment-1385162708.
I'll update it asap.
@carlosapaduarte, updated per your request in https://github.com/act-rules/act-rules.github.io/issues/1999#issuecomment-1385162708.
Ready for review
TODO: create very simple static demo pages for failed examples that reflect the link acc name (Contact Us).
Note: we still need to update the context definition to handle scenarios where the meaning of the sentence make the purpose of the link clear without ambiguity (with a different PR?).
e.g.
<div><a href="#">contact us</a> via chat, or <a href="#">contact us</a> via email.</div>
Punctuation might be covered by the note I've added in "ambiguous to users in general" definition, but link might be enough meaningful and distinguishable within its context also without punctuation.
@WilcoFiers There are multiple instances of the test checking the existence of the resources failing in this rule. Giacomo added test cases with query parameters in the URL (following Jean-Yves suggestion), and now the test complains the resource does not exist. The resource is there, but without the query parameter, of course. Can you update the test to not look at query parameters in the URL of the resource?
@giacomo-petri It seems the tests are failing because you're not in the contributors list. I suggest you do so in a separate PR (since it affects all of yours…), see, e.g. #2048 (plus a typo on "disambiguated" 🤔 )
This PR is in call for review for 2 weeks. Review ends Sept 29th.
I will promptly go through all the feedback. Regarding your specific point:
I don't mind having one or two examples of images in here. I don't think the examples should be exclusively about images. The far more common scenario is with just link text. I think the examples we had before did a better job of demonstrating that problem than the new examples do.
It's worth noting that the earlier examples posed a challenge for users in general, which is an exception for the 2.4.4 criterion evaluation.
Honestly, I never seen an example of a couple of links with identical accessible names and context serving different purposes without being ambiguous to users in general, except for the ones where something visual (background, icon, image) is making clear the difference.
@WilcoFiers,
Only query parameters topic is left out (pending agenda), then we should be ready to go for the call for review :D
Call for Review ends on February 23rd.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.
:white_check_mark: giacomo-petri
:x: Carlos Duarte
@Jym77
this seems to be ready to merge.
Call for review has ended, merging.
Our current failed scenarios are ambiguous to users in general, which conflicts with 1st assumption. With these changes I've:
Closes issue(s): #1999
Need for Call for Review: This will require a 2 weeks Call for Review I'm note sure about the timing here as multiple failed cases have been updated. I've set 2 weeks but feel free to update if needed.
Pull Request Etiquette
When creating PR:
develop
branch (left side).After creating PR:
Rule
,Definition
orChore
.When merging a PR:
How to Review And Approve