Integration - merge recent frontend highlighter and scanner change branches - #1066
Conversation
…the old scan runner in iframe checking contexts
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This change allows the frontend highlighter app to access the scanner bundle for enhanced functionality.
Refactor the scanning logic to improve readability and maintainability. Introduce helper methods for kicking off the scan, running the scan, and saving results. Enhance error handling and user feedback during the scanning process.
This update introduces new fields in the database schema to support enhanced accessibility features. [PRO-191]
This function extracts relevant details from a violation object, including selector, ancestry, xpath, and landmark information. [PRO-192]
This change introduces additional selectors (selector, ancestry, xpath) to the Insert_Rule_Data method, improving the flexibility and functionality of rule data insertion. [PRO-193]
Enhanced the insert method to accept additional parameters for selectors and ancestry, improving the flexibility of rule insertion. [PRO-193]
…n/merge-recent-frontend-highlighter-and-scanner-change-branches
…e' into william/integration/merge-recent-frontend-highlighter-and-scanner-change-branches
|
Caution Review failedThe pull request is closed. WalkthroughSelector-related fields ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FrontendApp
participant PageScanner
participant REST_API
participant InsertRuleData
participant Database
User->>FrontendApp: Initiate highlight/scan
FrontendApp->>PageScanner: runAccessibilityScan()
PageScanner->>PageScanner: Perform scan, collect violations with selector fields
PageScanner-->>FrontendApp: Return violations (with selector, ancestry, xpath)
FrontendApp->>REST_API: POST scan results (includes selector fields)
REST_API->>InsertRuleData: insert(post, rule, ..., selectors)
InsertRuleData->>Database: Insert record with selector, ancestry, xpath
Database-->>InsertRuleData: Confirmation
InsertRuleData-->>REST_API: Success
REST_API-->>FrontendApp: Scan results saved
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✨ 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 (
|
There was a problem hiding this comment.
Summary of Changes
Hello @pattonwebz, 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 integrates recent work on the frontend accessibility highlighter and the page scanner, enabling on-demand accessibility scans directly from the frontend. It significantly enhances the detail of stored violation data by capturing CSS selectors, element ancestry, and XPath, providing a more robust foundation for identifying and resolving accessibility issues.
Highlights
- Frontend-Initiated Accessibility Scanning: The frontend highlighter application can now dynamically load the accessibility scanner and initiate a scan of the current page directly from the user interface. This includes logic to retry fetching results after a scan is triggered.
- Enhanced Violation Data Capture: The system now captures and stores more granular details about accessibility violations, specifically including the CSS selector, element ancestry (a list of parent selectors), and XPath for each identified issue. This provides more precise location information for debugging and fixing.
- Database and API Integration for New Data: The database schema has been updated to include new columns for
selector,ancestry, andxpath. Correspondingly, the REST API endpoint for submitting scan results and theInsert_Rule_Dataclass have been modified to accept, process, and persist this new detailed violation data. - Refactored Scanner Module: The core page scanner module (
src/pageScanner/index.js) has been refactored to expose its scanning functionality viawindow.runAccessibilityScan, allowing it to be called programmatically by the frontend highlighter. It also includes improved timeout handling and helper functions for processing violation data.
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
The pull request integrates frontend highlighter and scanner changes. It introduces new database fields, enhances the frontend highlighter app, and updates the page scanner. There are some areas for improvement, including typo correction, potential infinite loop prevention, and error message enhancement.
| } | ||
| } else if ( ! self._scanAttempted && response.data?.[ 0 ]?.code === -3 ) { | ||
| // Only try kickoffScan once per highlightAjax call | ||
| self._scanAttempted = true; | ||
| self.kickoffScan(); | ||
| // After kickoffScan, try highlightAjax again, but only once | ||
| setTimeout( () => { | ||
| self.highlightAjax().then( resolve ).catch( reject ); | ||
| }, 5000 ); // Wait 5s for scan to complete. |
There was a problem hiding this comment.
The highlightAjax function retries the scan with a 5-second timeout. If the scan consistently fails or takes longer than 5 seconds, this could lead to an infinite loop and degrade performance. Consider adding a maximum retry count or implementing exponential backoff to prevent this.1
} else if ( ! self._scanAttempted && response.data?.[ 0 ]?.code === -3 ) {
// Only try kickoffScan once per highlightAjax call
self._scanAttempted = true;
self.kickoffScan();
// After kickoffScan, try highlightAjax again, but only once
setTimeout( () => {
self.highlightAjax().then( resolve ).catch( reject );
}, 5000 ); // Wait 5s for scan to complete.Style Guide References
Footnotes
| // eslint-disable-next-line no-unused-vars | ||
| const onDone = ( violations = [], errorMsgs = [], error = false ) => { |
There was a problem hiding this comment.
The onDone function has an unused parameter error = false. Remove the parameter to avoid confusion and improve code clarity.1
Style Guide References
| // eslint-disable-next-line no-unused-vars | |
| const onDone = ( violations = [], errorMsgs = [], error = false ) => { | |
| // eslint-disable-next-line no-unused-vars | |
| const onDone = ( violations = [], errorMsgs = [] ) => { |
Checklist
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests