-
Notifications
You must be signed in to change notification settings - Fork 8
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
Workaround lighthouse build #1809
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rajsite
requested review from
atmgrifter00,
msmithNI and
jattasNI
as code owners
February 7, 2024 22:15
@mollykreis Could you review this PR for re-enabling the lighthouse accessibility check (and if that seems like a good idea) |
mollykreis
approved these changes
Feb 7, 2024
Bypassing owners (mostly OOO) and buddied with Molly |
1 task
jattasNI
added a commit
that referenced
this pull request
Aug 14, 2024
# Pull Request ## π€¨ Rationale Resolves #280. Thanks to some recent fixes, the accessibility score for the Angular app is now back above 0.9 (it's 93%). We'd like to prevent regressions by ratcheting the `minScore` back to where it was originally before #1809. ## π©βπ» Implementation `0.8` β‘οΈ `0.9` in angular example app lighthouse config ## π§ͺ Testing The Lighthouse step passed in the [main / build (push) PR check](https://github.com/ni/nimble/actions/runs/10394512684/job/28784465522?pr=2346) and the [Lighthouse report looks sane](https://storage.googleapis.com/lighthouse-infrastructure.appspot.com/reports/1723668148618-11515.report.html). ## β Checklist <!--- Review the list and put an x in the boxes that apply or ~~strike through~~ around items that don't (along with an explanation). --> - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request
π€¨ Rationale
Disables the Card in the Angular and Blazor example apps as it breaks axe-core tooling used by lighthouse, see: #1650
π©βπ» Implementation
Disabled the nimble-card in the Angular and Blazor apps
Re-enabled the accessibility lighthouse error. The thought is that with this change the accessibility score will be reliably calculated and not cause intermittent blocking of merges, only blocks on merges that further regress the score. If it is not the case and lighthouse becomes unreliable calculating the score we can disable it again.
Removed the exisiting comment for an issue to get us back to 0.9 score as there are multiple issues impacting our score now.
π§ͺ Testing
Relying on lighthouse CI execution.
β Checklist