fix: a11y contrast issues#3722
Conversation
|
@jonas-hoebenreich is attempting to deploy a commit to the formbricks Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes primarily involve updates to the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
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)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
github-actions
bot
added
bug
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/web/app/s/[surveyId]/components/LegalFooter.tsx (2)
Line range hint
23-25: Improve accessibility for keyboard and screen reader users.While the color contrast has been addressed, there are a few more accessibility improvements we can make:
Remove
tabIndex={-1}from all links. This attribute prevents users from focusing on these links using the keyboard, which hinders accessibility.Add descriptive
aria-labelattributes to the links to provide more context for screen reader users.Consider adding a
role="contentinfo"to the maindivto semantically identify it as a footer.Here's a suggested implementation for one of the links (apply similar changes to others):
- <Link href={IMPRINT_URL} target="_blank" className="hover:underline" tabIndex={-1}> + <Link href={IMPRINT_URL} target="_blank" className="hover:underline" aria-label="View Imprint in a new tab"> Imprint </Link>And for the main
div:- <div className="absolute bottom-0 z-[1500] h-10 w-full"> + <div className="absolute bottom-0 z-[1500] h-10 w-full" role="contentinfo">Also applies to: 29-31, 36-41
Line range hint
24-24: Consider implementing internationalization (i18n).The component currently uses hardcoded English text for the links ("Imprint", "Privacy Policy", "Report Survey"). To improve the component's flexibility and support multiple languages, consider implementing internationalization.
Here's a basic example of how you might implement this using a hypothetical
useTranslationhook:+import { useTranslation } from 'your-i18n-library'; export const LegalFooter = ({ IMPRINT_URL, PRIVACY_URL, IS_FORMBRICKS_CLOUD, surveyUrl, }: LegalFooterProps) => { + const { t } = useTranslation(); if (!IMPRINT_URL && !PRIVACY_URL && !IS_FORMBRICKS_CLOUD) return null; return ( // ... - Imprint + {t('imprint')} // ... - Privacy Policy + {t('privacyPolicy')} // ... - Report Survey + {t('reportSurvey')} // ... ); };This change would allow for easy translation of the link text into multiple languages, improving the component's global usability.
Also applies to: 30-30, 39-39
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- apps/web/app/s/[surveyId]/components/LegalFooter.tsx (1 hunks)
- packages/surveys/src/styles/global.css (2 hunks)
🧰 Additional context used
🔇 Additional comments (4)
apps/web/app/s/[surveyId]/components/LegalFooter.tsx (1)
20-20: Improved color contrast, but verification needed.The change from
text-slate-400totext-slate-500is a step in the right direction to address the accessibility concerns mentioned in the PR objectives. This modification likely increases the contrast ratio, which is crucial for improving readability.However, to ensure full compliance with WCAG 2 AA standards, we need to verify that the new contrast ratio meets or exceeds the minimum requirement of 4.5:1.
To verify the contrast ratio:
- Determine the exact color values for
text-slate-500and the background color.- Use a contrast checker tool (like WebAIM's Contrast Checker) to calculate the ratio.
- Document the results in a comment for future reference.
Here's a script to help retrieve the color values:
After running this script, please manually check the contrast ratio using a contrast checker tool and document the results.
packages/surveys/src/styles/global.css (3)
78-78: Approve the color change for improved contrast.The change from
var(--slate-400)tovar(--slate-500)for--fb-signature-text-coloris a step in the right direction to address the accessibility contrast issues mentioned in the PR objectives and issue #3691.To ensure this change meets the WCAG 2 AA minimum contrast ratio of 4.5:1, please verify the contrast ratio using a tool like WebAIM's Contrast Checker (https://webaim.org/resources/contrastchecker/). Input the RGB values for
--slate-500(100, 116, 139) and the background color to confirm the new contrast ratio.
111-116: Approve formatting changes and scrollbar adjustments.The formatting changes in the
.fb-no-scrollbarclass improve code readability. The explicitscrollbar-colorsetting ensures consistency across browsers.Please verify the visual impact of
scrollbar-width: thinon Firefox browsers to ensure it aligns with the intended design. You can use Firefox's responsive design mode or a physical Firefox browser for testing.Also applies to: 123-123, 127-127
Line range hint
1-127: Summary: Accessibility improvements and code readability enhancements.The changes in this file align well with the PR objectives of addressing accessibility contrast issues. The adjustment to
--fb-signature-text-colorshould improve contrast, potentially resolving the issues mentioned in #3691 and #3689. The formatting changes in the.fb-no-scrollbarclass enhance code readability without altering core functionality.To ensure full compliance with accessibility standards:
- Verify the new contrast ratio meets WCAG 2 AA requirements (4.5:1) using a contrast checker tool.
- Test the visual impact of
scrollbar-width: thinon Firefox browsers.Overall, these changes represent a positive step towards improving the application's accessibility and maintainability.
jonas-hoebenreich
changed the title
mattinannt
left a comment
mattinannt
left a comment
There was a problem hiding this comment.
@jonas-hoebenreich thanks for the a11y fix :-)
|
Awarding jonas-hoebenreich: 50 points 🕹️ Well done! Check out your new contribution on oss.gg/jonas-hoebenreich |
2 participants
What does this PR do?
Fixes #3691 & #3689
How should this be tested?
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Bug Fixes
Style