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

Chore/merge main@bdfb4cc #807

Merged
merged 27 commits into from Sep 22, 2021
Merged

Chore/merge main@bdfb4cc #807

merged 27 commits into from Sep 22, 2021

Conversation

julien-deramond
Copy link
Member

@julien-deramond julien-deramond commented Sep 20, 2021

Get Bootstrap commits from 94c80ff613dce6620591acd7f9930ce8f40c5ed0 (Sept. 7) to bdfb4cc54d29c0c7bcd7944d3c8de2e1cd41bb6c (Sept. 15).

Dear reviewer, please pay attention to twbs/bootstrap@94c80ff which had some impacts on Boosted (-> d85ae3b)

@julien-deramond julien-deramond added chore merge Sync with Bootstrap v5 labels Sep 20, 2021
@julien-deramond julien-deramond added this to In progress in v5.2.0 via automation Sep 20, 2021
@julien-deramond
Copy link
Member Author

julien-deramond commented Sep 20, 2021

Bootstrap doesn't run pa11y yet. So I have to dig into the following report: https://github.com/Orange-OpenSource/Orange-Boosted-Bootstrap/suites/3813666741/artifacts/94282101; and maybe push a fix to Bootstrap 🤷

Error: Required ARIA attributes must be provided (https://dequeuniversity.com/rules/axe/3.5/aria-required-attr?application=axeAPI)
aria-required-attr
<input class="form-check-input" type="checkbox" role="switch" id="flexSwitchCheckDefault"> (select with "#flexSwitchCheckDefault")

Error: Required ARIA attributes must be provided (https://dequeuniversity.com/rules/axe/3.5/aria-required-attr?application=axeAPI)
aria-required-attr
<input class="form-check-input" type="checkbox" role="switch" id="flexSwitchCheckChecked" checked=""> (select with "#flexSwitchCheckChecked")

Error: Required ARIA attributes must be provided (https://dequeuniversity.com/rules/axe/3.5/aria-required-attr?application=axeAPI)
aria-required-attr
<input class="form-check-input" type="checkbox" role="switch" id="flexSwitchCheckDisabled" disabled=""> (select with "#flexSwitchCheckDisabled")

Error: Required ARIA attributes must be provided (https://dequeuniversity.com/rules/axe/3.5/aria-required-attr?application=axeAPI)
aria-required-attr
<input class="form-check-input" type="checkbox" role="switch" id="flexSwitchCheckCheckedDisabled" checked="" disabled=""> (select with "#flexSwitchCheckCheckedDisabled")

@julien-deramond julien-deramond removed the request for review from Nurovek September 20, 2021 06:33
@julien-deramond julien-deramond marked this pull request as draft September 20, 2021 06:33
@julien-deramond
Copy link
Member Author

pa11y is now happy but linkinator is not:

[...]
"file:/home/runner/work/Orange-Boosted-Bootstrap/Orange-Boosted-Bootstrap/_site/docs/5.1/forms/checks-radios/index.html":561.3-561.137: error: Attribute "aria-checked" not allowed on element "input" at this point.
"file:/home/runner/work/Orange-Boosted-Bootstrap/Orange-Boosted-Bootstrap/_site/docs/5.1/forms/checks-radios/index.html":561.3-561.137: info warning: The "aria-checked" attribute should not be used on an "input" element which has a "type" attribute whose value is "checkbox".

@julien-deramond
Copy link
Member Author

julien-deramond commented Sep 20, 2021

Got a discussion with the Orange a11y community and it seems that aria-checked is useful with a switch role (source: https://www.w3.org/TR/wai-aria-practices-1.1/#naming_with_aria-labelledby). So it may be rather a problem with Axe. And so I've added a specific rule for pa11y analysis to avoid checking .form-check.form-switch content.

Edit: Didn't see it but some info here too:

@julien-deramond
Copy link
Member Author

We begin to have several things ignored by pa11y (see "hideElements": ".bd-search, [id*='tarteaucitron'], #TableOfContents, .text-primary, .navbar-light .navbar-brand, .accordion-button:not(.collapsed), .active, [aria-current], select:disabled, [disabled] label, [disabled] + label, .modal, .bd-example nav, .badge.rounded-pill.bg-info.text-white, .exclude-from-pa11y-analysis, a.disabled, .form-check.form-switch",). Maybe it would be interesting to write somewhere (Wiki?) what each rule fixes?

@julien-deramond julien-deramond marked this pull request as ready for review September 20, 2021 10:12
Copy link
Contributor

@Nurovek Nurovek left a comment

Choose a reason for hiding this comment

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

Any reason on why Bootstrap chose to be more explicit with parameters in functions (e -> event) ?

Other than that, I've checked behaviours for components that are modified and it seems ok for me.

@julien-deramond
Copy link
Member Author

Any reason on why Bootstrap chose to be more explicit with parameters in functions (e -> event) ?

Probably just for readability.

@julien-deramond julien-deramond merged commit bf40af6 into main Sep 22, 2021
@julien-deramond julien-deramond deleted the chore/merge-main@bdfb4cc branch September 22, 2021 09:36
v5.2.0 automation moved this from In progress to Done Sep 22, 2021
@julien-deramond julien-deramond removed this from Done in v5.2.0 Oct 6, 2021
@julien-deramond julien-deramond added this to Done in v5.1.2 Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore merge Sync with Bootstrap v5
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants