Added Landmark Location - #1028
Conversation
WalkthroughThis update introduces support for associating accessibility violations with their nearest semantic landmarks in both backend storage and frontend display. It adds landmark data to database schemas, adjusts data insertion and retrieval logic, updates admin and frontend interfaces to display and highlight landmarks, and modifies related tests and styles. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FrontendHighlighter
participant Backend (REST/API)
participant Database
User->>FrontendHighlighter: Loads page with edac_landmark param
FrontendHighlighter->>FrontendHighlighter: Decodes selector, finds element
FrontendHighlighter->>FrontendHighlighter: Highlights landmark, adds label
User->>Backend (REST/API): Triggers scan or requests details
Backend (REST/API)->>Database: Query/access with landmark columns
Database-->>Backend (REST/API): Returns violations with landmark info
Backend (REST/API)-->>User: Returns/display violations with landmark links
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were identified. All code changes relate directly to the objectives in the linked issues. Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @SteveJonesDev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the accessibility checker's capabilities by integrating landmark region information into the violation reporting and storage mechanism. It introduces new database fields, updates backend data processing to handle this new information, and adds frontend logic to identify and associate violations with their respective semantic landmark elements, thereby improving the contextual understanding of accessibility issues.
Highlights
- Database Schema Extension: The database schema for rule data has been updated to include new columns (
landmarkandlandmark_selector) to store information about the semantic landmark region associated with a violation. - Backend Data Handling: The
Insert_Rule_Dataclass and the REST API endpoint responsible for processing scan results have been modified to accept, sanitize, and persist the new landmark data when recording accessibility violations. - Frontend Landmark Detection: New JavaScript logic has been introduced to detect the closest semantic landmark (e.g.,
main,header,footer,nav) for a given HTML element and generate a unique CSS selector for it. - Enhanced Violation Reporting: The frontend scanning logic now includes the detected landmark type and its unique selector as part of the violation record, providing more contextual information for each identified accessibility issue.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces landmark location tracking to the accessibility checker, enhancing its functionality. The changes include PHP backend updates for data storage and JavaScript frontend enhancements for landmark detection. The review suggests improvements for code maintainability, robustness, and clarity.
| item.violations.forEach( ( violation ) => { | ||
| if ( violation.result === 'failed' ) { | ||
| const selector = violation.node.selector; | ||
| const html = document.querySelector( selector )?.outerHTML; | ||
| const landmark = getLandmarkForSelector( selector ); | ||
| violations.push( { | ||
| selector: violation.node.selector, | ||
| html: document.querySelector( violation.node.selector ).outerHTML, | ||
| selector, | ||
| html, | ||
| ruleId: item.id, | ||
| impact: item.impact, | ||
| tags: item.tags, | ||
| landmark: landmark.type, | ||
| landmarkSelector: landmark.selector, | ||
| } ); | ||
| } | ||
| } ); |
There was a problem hiding this comment.
Consider extracting the violation processing logic into a helper function to reduce code duplication and improve maintainability.
function processAndPushViolation(item, violationData, violations) {
const selector = violationData.node.selector;
const element = document.querySelector(selector);
if (!element) {
return;
}
const html = element.outerHTML;
const landmark = getLandmarkForSelector(selector);
violations.push({
selector,
html,
ruleId: item.id,
impact: item.impact,
tags: item.tags,
landmark: landmark.type,
landmarkSelector: landmark.selector,
});
}
rules.forEach((item) => {
item.violations.forEach((violation) => {
if (violation.result === 'failed') {
processAndPushViolation(item, violation, violations);
}
});| item.incomplete.forEach( ( incompleteItem ) => { | ||
| const selector = incompleteItem.node.selector; | ||
| const html = document.querySelector( selector )?.outerHTML; | ||
| const landmark = getLandmarkForSelector( selector ); | ||
| violations.push( { | ||
| selector: incompleteItem.node.selector, | ||
| html: document.querySelector( incompleteItem.node.selector ).outerHTML, | ||
| selector, | ||
| html, | ||
| ruleId: item.id, | ||
| impact: item.impact, | ||
| tags: item.tags, | ||
| landmark: landmark.type, | ||
| landmarkSelector: landmark.selector, | ||
| } ); | ||
| } ); |
There was a problem hiding this comment.
Consider extracting the violation processing logic into a helper function to reduce code duplication and improve maintainability.
item.incomplete.forEach((incompleteItem) => {
processAndPushViolation(item, incompleteItem, violations);
});
}
});| * @param string|null $landmark The landmark type (main, header, footer, nav), optional. | ||
| * @param string|null $landmark_selector The landmark selector, optional. |
There was a problem hiding this comment.
Consider adding scalar type hints to the $landmark and $landmark_selector parameters for better type safety and code clarity. Use ?string since they are nullable.
* @param string|null $landmark The landmark type (main, header, footer, nav), optional.
* @param string|null $landmark_selector The landmark selector, optional.…mproving highlight functionality
|
The POT file has changed. Please update the POT file in your PR to keep translations up to date. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
includes/classes/class-rest-api.php (1)
383-384: Use the null coalescing operator (??) for more concise and readable code.$landmark = $violation['landmark'] ?? null; $landmark_selector = $violation['landmarkSelector'] ?? null;admin/class-insert-rule-data.php (1)
28-29: Add scalar type hints to the landmark parameters for better type safety.The past review comment about adding
?stringtype hints to the$landmarkand$landmark_selectorparameters is still valid.Apply this diff to add the type hints:
- * @param string|null $landmark The landmark type (main, header, footer, nav), optional. - * @param string|null $landmark_selector The landmark selector, optional. + * @param string|null $landmark The landmark type (main, header, footer, nav), optional. + * @param string|null $landmark_selector The landmark selector, optional. */ - public function insert( object $post, string $rule, string $ruletype, string $rule_obj, $landmark = null, $landmark_selector = null ) { + public function insert( object $post, string $rule, string $ruletype, string $rule_obj, ?string $landmark = null, ?string $landmark_selector = null ) {Also applies to: 34-34
src/pageScanner/index.js (2)
138-145: Use CSS.escape() for class names to handle special characters.The current implementation could fail if class names contain special CSS characters.
Apply this diff to properly escape class names:
if ( current.className ) { - const classes = current.className.trim().split( /\s+/ ) + const classes = current.className.trim().split( /\s+/ ).map(cls => CSS.escape(cls)) .filter( ( cls ) => ! cls.match( /^(wp-|js-|css-|generated-|dynamic-)/ ) ) // Filter out common dynamic classes .slice( 0, 2 ); // Limit to first 2 classes for stability if ( classes.length > 0 ) { selector += `.${ classes.join( '.' ) }`; } }
210-225: Extract violation processing logic to reduce code duplication.The logic for processing violations is duplicated between regular violations and incomplete results.
Consider extracting the common logic into a helper function:
+function processViolationNode(node, item, violations) { + const selector = node.selector; + const element = document.querySelector(selector); + + if (!element) { + return; + } + + const html = element.outerHTML; + const landmark = getLandmarkForSelector(selector); + + violations.push({ + selector, + html, + ruleId: item.id, + impact: item.impact, + tags: item.tags, + landmark: landmark.type, + landmarkSelector: landmark.selector, + }); +} rules.forEach( ( item ) => { //Build an array of the dom selectors and ruleIDs for violations/failed tests item.violations.forEach( ( violation ) => { if ( violation.result === 'failed' ) { - const selector = violation.node.selector; - const html = document.querySelector( selector )?.outerHTML; - const landmark = getLandmarkForSelector( selector ); - violations.push( { - selector, - html, - ruleId: item.id, - impact: item.impact, - tags: item.tags, - landmark: landmark.type, - landmarkSelector: landmark.selector, - } ); + processViolationNode(violation.node, item, violations); } } ); // Handle incomplete results for form-field-multiple-labels only. if ( item.id === 'form-field-multiple-labels' ) { // Allow incomplete results for this rule. item.incomplete.forEach( ( incompleteItem ) => { - const selector = incompleteItem.node.selector; - const html = document.querySelector( selector )?.outerHTML; - const landmark = getLandmarkForSelector( selector ); - violations.push( { - selector, - html, - ruleId: item.id, - impact: item.impact, - tags: item.tags, - landmark: landmark.type, - landmarkSelector: landmark.selector, - } ); + processViolationNode(incompleteItem.node, item, violations); } ); } } );Also applies to: 229-242
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
accessibility-checker.php(1 hunks)admin/class-ajax.php(4 hunks)admin/class-insert-rule-data.php(3 hunks)admin/class-update-database.php(1 hunks)includes/classes/class-rest-api.php(1 hunks)src/admin/sass/accessibility-checker-admin.scss(2 hunks)src/frontendHighlighterApp/index.js(5 hunks)src/pageScanner/index.js(2 hunks)tests/phpunit/Admin/InsertRuleDataTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: SteveJonesDev
PR: equalizedigital/accessibility-checker#921
File: src/pageScanner/checks/linked-image-alt-present.js:43-50
Timestamp: 2025-04-15T02:30:38.897Z
Learning: In the Accessibility Checker plugin, accessibility rules are separated by specific concerns. For example, linked images have separate rules for checking: (1) missing alt attributes and (2) empty alt attributes. The rule `linked_image_alt_present` specifically checks for the presence of alt attributes on linked images, while a separate rule `img_linked_alt_empty` handles validation of empty alt attributes. Suggestions should respect this separation of concerns.
admin/class-ajax.php (5)
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#927
File: src/pageScanner/checks/img-alt-missing-check.js:35-37
Timestamp: 2025-04-18T14:27:18.140Z
Learning: In the Accessibility Checker plugin, the img_alt_missing rule specifically checks for missing alt attributes on images and image inputs, while empty alt attributes are handled by a separate rule. Each accessibility concern is deliberately separated into individual rules.
Learnt from: SteveJonesDev
PR: equalizedigital/accessibility-checker#921
File: src/pageScanner/checks/linked-image-alt-present.js:43-50
Timestamp: 2025-04-15T02:30:38.897Z
Learning: In the Accessibility Checker plugin, accessibility rules are separated by specific concerns. For example, linked images have separate rules for checking: (1) missing alt attributes and (2) empty alt attributes. The rule `linked_image_alt_present` specifically checks for the presence of alt attributes on linked images, while a separate rule `img_linked_alt_empty` handles validation of empty alt attributes. Suggestions should respect this separation of concerns.
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#927
File: src/pageScanner/checks/img-alt-missing-check.js:32-32
Timestamp: 2025-04-18T14:27:49.512Z
Learning: In the Accessibility Checker plugin, the img_alt_missing rule specifically checks for missing alt attributes (whether the attribute exists at all), while empty alt attributes (alt="") are handled by a separate rule. This separation of concerns is by design.
Learnt from: SteveJonesDev
PR: equalizedigital/accessibility-checker#921
File: src/pageScanner/checks/linked-image-alt-present.js:43-50
Timestamp: 2025-04-15T02:30:38.897Z
Learning: In the Accessibility Checker plugin, there are separate rules for different aspects of image accessibility. The rule `linked_image_alt_present` specifically checks for the presence of alt attributes on linked images, while a separate rule handles validation of empty alt attributes.
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#881
File: src/pageScanner/checks/duplicate-form-label-check.js:51-55
Timestamp: 2025-04-08T21:45:57.372Z
Learning: The Accessibility Checker intentionally flags multiple IDs in aria-labelledby as a failure, even though it's technically allowed in the ARIA spec. This design decision was made because multiple IDs can cause confusion with some screen readers, and the tool aims to discourage any form of duplicate labelling to ensure maximum compatibility.
src/pageScanner/index.js (1)
Learnt from: SteveJonesDev
PR: equalizedigital/accessibility-checker#880
File: src/pageScanner/rules/video-present.js:3-3
Timestamp: 2025-04-08T21:36:48.803Z
Learning: In the accessibility-checker, the broad selector pattern in video-present.js (`'video, iframe, object, source, [src], [class], [role]'`) is intentional, as it allows the JavaScript evaluation function in video-detected.js to perform case-insensitive matching and sophisticated pattern detection that CSS selectors cannot replicate.
src/frontendHighlighterApp/index.js (1)
Learnt from: SteveJonesDev
PR: equalizedigital/accessibility-checker#880
File: src/pageScanner/rules/video-present.js:3-3
Timestamp: 2025-04-08T21:36:48.803Z
Learning: In the accessibility-checker, the broad selector pattern in video-present.js (`'video, iframe, object, source, [src], [class], [role]'`) is intentional, as it allows the JavaScript evaluation function in video-detected.js to perform case-insensitive matching and sophisticated pattern detection that CSS selectors cannot replicate.
🧬 Code Graph Analysis (2)
includes/classes/class-rest-api.php (1)
admin/class-insert-rule-data.php (2)
Insert_Rule_Data(17-156)insert(34-155)
src/frontendHighlighterApp/index.js (2)
src/pageScanner/index.js (4)
selector(128-128)role(49-49)role(111-111)tagName(82-82)src/pageScanner/checks/has-ambiguous-text.js (1)
label(43-43)
🪛 Biome (1.9.4)
src/frontendHighlighterApp/index.js
[error] 934-934: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1054-1057: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1060-1062: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (7)
accessibility-checker.php (1)
43-43: LGTM: Database version bump is appropriate.The version increment from '1.0.3' to '1.0.5' correctly reflects the database schema changes that add landmark support.
tests/phpunit/Admin/InsertRuleDataTest.php (1)
40-41: LGTM: Test schema correctly updated to match production.The addition of
landmarkandlandmark_selectorcolumns maintains proper alignment between test and production database schemas.src/admin/sass/accessibility-checker-admin.scss (1)
624-624: LGTM: Grid layout properly updated for landmark column.The addition of a fourth 125px column maintains visual consistency while accommodating the new landmark information in the admin interface.
Also applies to: 634-634
admin/class-update-database.php (1)
51-52: LGTM: Database schema correctly updated for landmark support.The new nullable columns are appropriately typed -
varchar(20)for landmark types andtextfor CSS selectors. The NULL constraints ensure backward compatibility with existing records.includes/classes/class-rest-api.php (1)
386-386: LGTM: Landmark data properly passed to Insert_Rule_Data.The integration with the enhanced
Insert_Rule_Data::insert()method correctly provides the new landmark parameters, enabling storage of landmark context for accessibility violations.admin/class-ajax.php (1)
308-308: LGTM! Well-implemented landmark display with proper security and accessibility.The implementation correctly:
- Retrieves landmark data from the database
- Displays landmark information with appropriate links when both landmark and selector are available
- Uses nonce for security in the landmark URLs
- Includes proper ARIA labels and translatable strings for accessibility
- Handles all edge cases (landmark with/without selector, no landmark)
Also applies to: 419-421, 469-494
src/frontendHighlighterApp/index.js (1)
906-1006: Excellent implementation of landmark highlighting with robust fallback strategies.The implementation includes:
- Proper error handling with try-catch blocks
- Multiple fallback strategies for finding elements when the original selector fails
- Well-styled landmark labels with appropriate positioning
- Silent failure approach that doesn't disrupt the user experience
- Proper cleanup integration with existing highlight removal
…ting to landmarks
…arameters in insert method
…tor in violation handling
…ved accessibility
|
The POT file has changed. Please update the POT file in your PR to keep translations up to date. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/frontendHighlighterApp/index.js (1)
1057-1069: Wrap switch case declarations in blocks to prevent scope issues.Variables declared in switch cases can be accessed by other cases, which can lead to unexpected behavior.
Apply this diff to properly scope the variables:
case 'section': { - // Check if section has accessible name - const hasAccessibleName = element.getAttribute( 'aria-label' ) || - element.getAttribute( 'aria-labelledby' ) || - element.querySelector( 'h1, h2, h3, h4, h5, h6' ); - return hasAccessibleName ? 'Region' : 'Section'; + { + // Check if section has accessible name + const hasAccessibleName = element.getAttribute( 'aria-label' ) || + element.getAttribute( 'aria-labelledby' ) || + element.querySelector( 'h1, h2, h3, h4, h5, h6' ); + return hasAccessibleName ? 'Region' : 'Section'; + } case 'form': - // Check if form has accessible name - const formHasAccessibleName = element.getAttribute( 'aria-label' ) || - element.getAttribute( 'aria-labelledby' ); - return formHasAccessibleName ? 'Form' : 'Form (unlabeled)'; + { + // Check if form has accessible name + const formHasAccessibleName = element.getAttribute( 'aria-label' ) || + element.getAttribute( 'aria-labelledby' ); + return formHasAccessibleName ? 'Form' : 'Form (unlabeled)'; + }
🧹 Nitpick comments (1)
src/frontendHighlighterApp/index.js (1)
902-1011: Consider refactoring for better maintainability and apply optional chaining.The landmark highlighting functionality works correctly, but consider these improvements:
- Use optional chaining (as suggested by static analysis):
-if ( fallback && fallback.trim() ) { +if ( fallback?.trim() ) {
- Consider breaking down this large method into smaller, focused functions for better maintainability:
decodeLandmarkSelector()findLandmarkElement()createLandmarkLabel()positionAndShowLandmark()The current implementation handles edge cases well and provides good fallback strategies for selector matching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
admin/class-ajax.php(4 hunks)admin/class-insert-rule-data.php(3 hunks)includes/classes/class-rest-api.php(1 hunks)src/frontendHighlighterApp/index.js(5 hunks)src/frontendHighlighterApp/sass/app.scss(1 hunks)src/pageScanner/index.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/frontendHighlighterApp/sass/app.scss
🚧 Files skipped from review as they are similar to previous changes (4)
- includes/classes/class-rest-api.php
- admin/class-ajax.php
- admin/class-insert-rule-data.php
- src/pageScanner/index.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: SteveJonesDev
PR: equalizedigital/accessibility-checker#921
File: src/pageScanner/checks/linked-image-alt-present.js:43-50
Timestamp: 2025-04-15T02:30:38.897Z
Learning: In the Accessibility Checker plugin, accessibility rules are separated by specific concerns. For example, linked images have separate rules for checking: (1) missing alt attributes and (2) empty alt attributes. The rule `linked_image_alt_present` specifically checks for the presence of alt attributes on linked images, while a separate rule `img_linked_alt_empty` handles validation of empty alt attributes. Suggestions should respect this separation of concerns.
src/frontendHighlighterApp/index.js (2)
Learnt from: SteveJonesDev
PR: equalizedigital/accessibility-checker#880
File: src/pageScanner/rules/video-present.js:3-3
Timestamp: 2025-04-08T21:36:48.803Z
Learning: In the accessibility-checker, the broad selector pattern in video-present.js (`'video, iframe, object, source, [src], [class], [role]'`) is intentional, as it allows the JavaScript evaluation function in video-detected.js to perform case-insensitive matching and sophisticated pattern detection that CSS selectors cannot replicate.
Learnt from: pattonwebz
PR: equalizedigital/accessibility-checker#881
File: src/pageScanner/checks/duplicate-form-label-check.js:51-55
Timestamp: 2025-04-08T21:45:57.372Z
Learning: The Accessibility Checker intentionally flags multiple IDs in aria-labelledby as a failure, even though it's technically allowed in the ARIA spec. This design decision was made because multiple IDs can cause confusion with some screen readers, and the tool aims to discourage any form of duplicate labelling to ensure maximum compatibility.
🪛 Biome (1.9.4)
src/frontendHighlighterApp/index.js
[error] 934-934: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: make-pot
🔇 Additional comments (6)
src/frontendHighlighterApp/index.js (6)
35-35: LGTM! Parameter handling follows established patterns.The landmark parameter addition is consistent with the existing URL parameter handling approach.
101-103: LGTM! Proper integration of landmark highlighting logic.The conditional logic correctly handles the landmark parameter while maintaining backward compatibility with existing issue highlighting.
249-251: LGTM! Proper cleanup integration.Adding landmark label cleanup when removing highlight buttons ensures a consistent clean state.
587-589: LGTM! Consistent cleanup logic.Adding landmark label cleanup when removing selected classes maintains visual consistency.
1018-1073: LGTM! Comprehensive landmark type detection logic.The method properly handles both ARIA roles and semantic HTML elements with appropriate fallbacks. The logic for distinguishing between labeled and unlabeled sections/forms is particularly well thought out.
1078-1090: LGTM! Thorough cleanup implementation.The method comprehensively removes landmark labels, classes, and attributes, ensuring no visual artifacts remain after cleanup.
|
The POT file has changed. Please update the POT file in your PR to keep translations up to date. |
…lizedigital/accessibility-checker into steve/try/landmark-location
|
The POT file has changed. Please update the POT file in your PR to keep translations up to date. |
|
The POT file has changed. Please update the POT file in your PR to keep translations up to date. |
pattonwebz
left a comment
pattonwebz
left a comment
There was a problem hiding this comment.
I have been testing this as part of further work (I based from this for the selector PR) and found no major blocking issues. Approving.
As a note in the PR I branched from here to create I made a few tweaks:
- extracted some logic to reusable helper for processing violations
- updated the rulecheck part of the query to also update landmark and the selectors when it is flagging an unchanged rule (the location might still have changed even if the markup hasn't).
2 participants
This pull request introduces significant enhancements to the accessibility checker by adding landmark detection and highlighting functionality. The changes include updates to the database schema, backend logic, and frontend UI to support landmarks like headers, footers, and navigation regions. Below are the most important changes grouped by theme:
Database and Backend Enhancements:
landmarkandlandmark_selectorfields for storing landmark information in theedac_rulestable. (admin/class-update-database.php, admin/class-update-database.phpR51-R52)Insert_Rule_Dataclass to handle landmarks, addinglandmarkandlandmark_selectorparameters to theinsertmethod and ensuring proper sanitization. (admin/class-insert-rule-data.php, [1] [2] [3]includes/classes/class-rest-api.php, includes/classes/class-rest-api.phpL383-R386)Frontend Highlighting Improvements:
highlightLandmarkmethod to visually highlight landmarks on the frontend, including creating labels and using fallback strategies for landmark selection. (src/frontendHighlighterApp/index.js, src/frontendHighlighterApp/index.jsR901-R1090)src/frontendHighlighterApp/index.js, [1] [2]UI and Styling Updates:
admin/class-ajax.php, [1] [2]src/admin/sass/accessibility-checker-admin.scss, [1] [2]Landmark Detection Logic:
src/pageScanner/index.js, src/pageScanner/index.jsR13-R164)Miscellaneous:
1.0.3to1.0.5to reflect the new features. (accessibility-checker.php, accessibility-checker.phpL43-R43)These changes collectively enhance the accessibility checker by improving its ability to detect, store, and highlight landmarks, making it easier for users to identify and address accessibility issues.
Fixes: https://linear.app/equalize-digital/issue/PRO-183/create-a-custom-database-column-to-store-the-issues-location
Fixes: https://linear.app/equalize-digital/issue/PRO-185/identify-the-landmark-location
Fixes: https://linear.app/equalize-digital/issue/PRO-184/add-a-location-column-to-the-details-panel-for-users-that-can-see-all
Fixes: #676
Fixes: #58
Fixes: #557
Summary by CodeRabbit
Summary by CodeRabbit
New Features
User Interface
Tests