test(urls-accessibility): add accessibility and scaling coverage#5105
test(urls-accessibility): add accessibility and scaling coverage#5105Subhooo5 wants to merge 2 commits into
Conversation
|
Hey @JhaSourav07 Test added successfully. All existing tests pass locally. Feel free to review, add necessary labels and merge it. 🫂🚀 |
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the PR. I have to request changes. You added an accessibility test suite for 'utils/urls.ts' testing ARIA relationships and focus visibility. A URL construction utility file does not render HTML or manage DOM focus, so it cannot have ARIA attributes. Please target accessibility tests only at valid UI components (.tsx).
8b7eab5 to
cd3f0e8
Compare
File updated successfully, All tests pass locally, feel free to review & add necessary labels. 🚀 |
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for putting this together. The changes are clean and the logic looks sound. The accessibility improvements and ARIA attribute coverage look great and will really help screen reader users. Approved!
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for putting this together. The code looks clean and solves the issue effectively.
I'm happy to approve this. Great job!
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I went through the changes and the overall approach looks good, but there are a few issues that should be addressed before this can be merged. Most of the concerns are related to correctness and maintainability.
- There are merge conflicts with the base branch. Please resolve them to ensure existing functionality isn't broken.
Once these issues are addressed, I'll be happy to take another look. Thanks again for the contribution.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution. It looks like there are merge conflicts with the base branch. Please rebase and resolve the conflicts so we can proceed with testing and merging. Thanks!
|
🚨 Hey @Subhooo5, the CI Pipeline is failing on this PR and it has been marked as Please fix the issues before this can be reviewed. Here's how: 1. Run checks locally before pushing: npm run format:check # Check Prettier formatting
npm run lint # Run ESLint
npm run typecheck # TypeScript type check
npm run test # Run unit tests (Vitest)
npm run build # Verify production build passes2. Auto-fix common issues: npm run format # Auto-fix formatting with Prettier
npm run lint -- --fix # Auto-fix lint errors where possible3. Check the full failure log here: Once you push a fix and the CI passes, the |
|
Hey @Aamod007 @JhaSourav07 Due to excessive confusion and mixups I closed this PR and have created a new fresh PR #5363 for issue #4661. Kindly review, add necessary labels and merge #5363 at the earliest to avoid further merge conflicts. 🫂🚀 |
Description
Fixes #4661
utils/urls.accessibility.test.tswith5accessibility-focused tests validatingARIA relationships,keyboard focus visibility,tooltip descriptions,tab ordering, andheading hierarchyaround dashboard URL interactions.Pillar
Checklist before requesting a review:
CONTRIBUTING.mdfile.localhost:3000/api/streak?user=YOUR_USERNAME).npm run formatandnpm run lintlocally and resolved all errors.