8000 R113 follow up by rcj-siteimprove · Pull Request #1592 · Siteimprove/alfa · GitHub
[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

R113 follow up #1592

Merged
merged 21 commits into from
Apr 5, 2024
Merged

R113 follow up #1592

merged 21 commits into from
Apr 5, 2024

Conversation

rcj-siteimprove
Copy link
Contributor
@rcj-siteimprove rcj-siteimprove commented Apr 2, 2024 •
edited
Loading

Follows up on #1589

We send more granular information about how the rule succeeded in the extended diagnostics and the spacing computation should now be correctly implemented.

The spacing still has a quadratic runtime, but I'm working on finding a solution using some kind of search tree. I might add it to this PR or in another follow up.

The image shows how the distance to the corners are calculated:
image

@changeset-bot changeset-bot
Copy link
changeset-bot bot commented Apr 2, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 1061bbf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 78 packages
Name Type
@siteimprove/alfa-rules Minor
@siteimprove/alfa-rectangle Minor
@siteimprove/alfa-dom Minor
@siteimprove/alfa-style Minor
@siteimprove/alfa-act Minor
@siteimprove/alfa-affine Minor
@siteimprove/alfa-applicative Minor
@siteimprove/alfa-aria Minor
@siteimprove/alfa-array Minor
@siteimprove/alfa-bits Minor
@siteimprove/alfa-branched Minor
@siteimprove/alfa-cache Minor
@siteimprove/alfa-callback Minor
@siteimprove/alfa-cascade Minor
@siteimprove/alfa-clone Minor
@siteimprove/alfa-collection Minor
@siteimprove/alfa-comparable Minor
@siteimprove/alfa-compatibility Minor
@siteimprove/alfa-continuation Minor
@siteimprove/alfa-css-feature Minor
@siteimprove/alfa-css Minor
@siteimprove/alfa-device Minor
@siteimprove/alfa-earl Minor
@siteimprove/alfa-either Minor
@siteimprove/alfa-emitter Minor
@siteimprove/alfa-encoding Minor
@siteimprove/alfa-equatable Minor
@siteimprove/alfa-flags Minor
@siteimprove/alfa-fnv Minor
@siteimprove/alfa-foldable Minor
@siteimprove/alfa-functor Minor
@siteimprove/alfa-future Minor
@siteimprove/alfa-generator Minor
@siteimprove/alfa-graph Minor
@siteimprove/alfa-hash Minor
@siteimprove/alfa-http Minor
@siteimprove/alfa-iana Minor
@siteimprove/alfa-iterable Minor
@siteimprove/alfa-json-ld Minor
@siteimprove/alfa-json Minor
@siteimprove/alfa-lazy Minor
@siteimprove/alfa-list Minor
@siteimprove/alfa-map Minor
@siteimprove/alfa-mapper Minor
@siteimprove/alfa-math Minor
@siteimprove/alfa-media Minor
@siteimprove/alfa-monad Minor
@siteimprove/alfa-network Minor
@siteimprove/alfa-option Minor
@siteimprove/alfa-parser Minor
@siteimprove/alfa-performance Minor
@siteimprove/alfa-predicate Minor
@siteimprove/alfa-promise Minor
@siteimprove/alfa-record Minor
@siteimprove/alfa-reducer Minor
@siteimprove/alfa-refinement Minor
@siteimprove/alfa-result Minor
@siteimprove/alfa-sarif Minor
@siteimprove/alfa-selective Minor
@siteimprove/alfa-selector Minor
@siteimprove/alfa-sequence Minor
@siteimprove/alfa-set Minor
@siteimprove/alfa-slice Minor
@siteimprove/alfa-string Minor
@siteimprove/alfa-table Minor
@siteimprove/alfa-test Minor
@siteimprove/alfa-thenable Minor
@siteimprove/alfa-thunk Minor
@siteimprove/alfa-time Minor
@siteimprove/alfa-toolchain Minor
@siteimprove/alfa-trampoline Minor
@siteimprove/alfa-tree Minor
@siteimprove/alfa-trilean Minor
@siteimprove/alfa-tuple Minor
@siteimprove/alfa-url Minor
@siteimprove/alfa-wcag Minor
@siteimprove/alfa-web Minor
@siteimprove/alfa-xpath Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@rcj-siteimprove rcj-siteimprove marked this pull request as ready for review April 2, 2024 11:45
@rcj-siteimprove rcj-siteimprove requested a review from a team as a code owner April 2, 2024 11:45
@rcj-siteimprove rcj-siteimprove self-assigned this Apr 2, 2024
@rcj-siteimprove rcj-siteimprove added the minor Backwards-compatible change that touches public API label Apr 2, 2024
Jym77
Jym77 requested changes Apr 2, 2024
View reviewed changes
packages/alfa-rules/src/common/diagnostic/with-bounding-box.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/common/diagnostic/with-bounding-box.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/common/diagnostic/with-bounding-box.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/common/diagnostic/with-bounding-box.ts Show resolved Hide resolved
packages/alfa-rules/src/sia-r113/rule.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/sia-r113/rule.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/sia-r113/rule.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/sia-r113/rule.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/sia-r113/rule.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/sia-r113/rule.ts Outdated Show resolved Hide resolved
@rcj-siteimprove
Copy link
Contributor Author

!pr extract

@rcj-siteimprove
Copy link
Contributor Author

!pr extract

@rcj-siteimprove rcj-siteimprove requested a review from Jym77 April 4, 2024 15:02
@rcj-siteimprove
Copy link
Contributor Author

@Jym77 I'm not sure I'm happy about the way I collect the neighbors by passing in and pushing to an array, but I couldn't think of any much better solution due to how the expectation function works. Please advise if you see a better way 😄

Jym77
Jym77 requested changes Apr 5, 2024
View reviewed changes
Copy link
Contributor
@Jym77 Jym77 left a comment

Choose a reason for hiding this comment

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

Looks good. Some smallish polish.

packages/alfa-rectangle/src/rectangle.ts Outdated Show resolved Hide resolved
packages/alfa-rectangle/src/rectangle.ts Outdated Show resolved Hide resolved
packages/alfa-rectangle/src/rectangle.ts Outdated Show resolved Hide resolved
packages/alfa-rectangle/src/rectangle.ts Outdated Show resolved Hide resolved
packages/alfa-rectangle/src/rectangle.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/common/diagnostic/with-bounding-box.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/sia-r113/rule.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/sia-r113/rule.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/sia-r113/rule.ts Outdated Show resolved Hide resolved
packages/alfa-rules/src/sia-r111/rule.ts Show resolved Hide resolved
rcj-siteimprove and others added 2 commits April 5, 2024 11:02
@rcj-siteimprove @Jym77
Apply suggestions from code review
948fab6 
Co-authored-by: Jean-Yves Moyen <jym@siteimprove.com>
@rcj-siteimprove
Refactor expectation
65b7911
@rcj-siteimprove
Copy link
Contributor Author

!pr extract

@rcj-siteimprove rcj-siteimprove requested a review from Jym77 April 5, 2024 09:42
@rcj-siteimprove rcj-siteimprove added this pull request to the merge queue Apr 5, 2024
Merged via the queue into main with commit 07deef6 Apr 5, 2024
6 checks passed
@rcj-siteimprove rcj-siteimprove deleted the r113-follow-up branch April 5, 2024 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jym77 Jym77 Jym77 approved these changes

Assignees

@rcj-siteimprove rcj-siteimprove

Labels
minor Backwards-compatible change that touches public API
Projects
⚙️ Alfa Development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rcj-siteimprove @Jym77
0