-
-
Notifications
You must be signed in to change notification settings - Fork 902
Enhanced button animation for more engaging user experience. #3988
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.
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
Enhanced button animation for more engaging user experience. #3988
Conversation
WalkthroughThe pull request updates the styling of the button component in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
We require all PRs to follow Conventional Commits specification.
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/buttons/Button.tsx (1)
65-68
: Consider a shorter transition durationThe 500ms transition duration might feel slow for a button hover effect. Consider shortening it to 300ms for a more responsive feel unless this duration is standardized across your application.
- const smallButtonClasses = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 + const smallButtonClasses = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-300- const classNames = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 ease-in-out + const classNames = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-300 ease-in-out🧰 Tools
🪛 ESLint
[error] 65-65: Insert
⏎···
(prettier/prettier)
[error] 67-67: Insert
⏎···
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 65-67: Insert
⏎···
prettier/prettier
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/buttons/Button.tsx
(1 hunks)
🧰 Additional context used
🪛 ESLint
components/buttons/Button.tsx
[error] 65-65: Insert ⏎···
(prettier/prettier)
[error] 67-67: Insert ⏎···
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
components/buttons/Button.tsx
[error] 65-67: Insert ⏎···
prettier/prettier
🔇 Additional comments (2)
components/buttons/Button.tsx (2)
60-61
: Improved button interaction stylingThe hover effect now reverses the button colors (white background with primary text), which provides better visual feedback to users when interacting with the button, aligning with the PR's goal of enhancing button animations.
60-61
:❓ Verification inconclusive
Verify color contrast for accessibility
Ensure there's sufficient contrast between the primary text color and the white background on hover for accessibility compliance.
🌐 Web query:
What is the recommended color contrast ratio for accessibility between text and background?
💡 Result:
The recommended color contrast ratio for accessibility between text and background depends on the text size and WCAG compliance level, with key guidelines as follows:
Minimum Requirements (WCAG Level AA)
- Normal text (smaller than 18pt/24px or 14pt/18.67px bold):
At least 4.5:1 contrast ratio with the background[2][5][6][8][15].- Large text (18pt/24px or larger, or 14pt/18.67px bold):
Minimum 3:1 contrast ratio[2][5][8][15].- Non-text elements (UI components, graphics):
3:1 contrast ratio for interactive states or critical visual information[2][6][11][16].Enhanced Requirements (WCAG Level AAA)
- Normal text: At least 7:1 contrast ratio[2][5][12][15].
- Large text: 4.5:1 contrast ratio[5][15][12].
Exceptions
- Logos/branding: No minimum requirement[6][7][11].
- Incidental/decorative text: Exempt from contrast rules[6][7][11].
Best Practices
- Avoid color-only indicators: Use additional cues like underlines for links or symbols for required fields[1][3].
- Test with tools: Use validators like WebAIM’s Contrast Checker or Accessible Web’s tools to verify ratios[5][12].
- Consider user needs: Very high contrast (e.g., pure black on white) may hinder readability for users with dyslexia[4].
For example:
- Passing combinations: Dark blue (#112e51) on white achieves 4.5:1[1].
- Failing combinations: Red (#FF0000) on white has a 4:1 ratio, below the 4.5:1 threshold[6].
Always prioritize readability for diverse audiences, including those with low vision or color deficiencies[3][9][15].
Citations:
- 1: http://web-accessibility.carnegiemuseums.org/design/color/
- 2: https://accessiblyapp.com/web-accessibility/color-contrast/
- 3: https://www.maine.edu/content-management/accessibility/color-ratio/
- 4: https://accessibility.huit.harvard.edu/use-sufficient-color-contrast
- 5: https://accessibleweb.com/color-contrast-checker/
- 6: https://webaim.org/articles/contrast/
- 7: https://www.w3.org/TR/UNDERSTANDING-WCAG20/visual-audio-contrast-contrast.html
- 8: https://www.boia.org/blog/designing-for-color-contrast-guidelines-for-accessibility
- 9: https://dequeuniversity.com/rules/axe/4.10/color-contrast
- 10: https://dequeuniversity.com/rules/axe/4.9/color-contrast
- 11: https://www.w3.org/TR/WCAG21/
- 12: https://webaim.org/resources/contrastchecker/
- 13: https://my.willamette.edu/site/digital-accessibility/guidelines/color-contrast
- 14: https://brand.ucla.edu/fundamentals/accessibility/color-type
- 15: https://www.w3.org/WAI/WCAG21/Understanding/contrast-minimum.html
- 16: https://m3.material.io/foundations/designing/color-contrast
Action Required: Confirm color contrast meets WCAG guidelines
Please verify that the hover state of the button—where the background switches to white and the text applies the primary-500 color—meets accessibility contrast requirements. Specifically:
- For normal text, ensure the contrast ratio is at least 4.5:1.
- For large text, a minimum ratio of 3:1 is required.
- If adhering to WCAG Level AAA standards, higher contrast ratios (7:1 for normal and 4.5:1 for large text) may be needed.
Consider using tools like WebAIM’s Contrast Checker for validation.
File: components/buttons/Button.tsx (Lines: 60-61)
const smallButtonClasses = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 | ||
ease-in-out rounded-md px-3 py-2 text-sm font-medium tracking-heading ${className || ''}`); | ||
const classNames = twMerge(`${bgClassName} ${textClassName} transition-all duration-500 ease-in-out | ||
const classNames = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 ease-in-out | ||
rounded-md px-4 py-3 text-md font-semibold tracking-heading ${className || ''}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix formatting issues
The static analysis tool and pipeline tests are failing due to formatting issues. You need to fix the indentation according to prettier rules.
- const smallButtonClasses = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500
- ease-in-out rounded-md px-3 py-2 text-sm font-medium tracking-heading ${className || ''}`);
- const classNames = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 ease-in-out
- rounded-md px-4 py-3 text-md font-semibold tracking-heading ${className || ''}`);
+ const smallButtonClasses = twMerge(
+ `${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 ease-in-out rounded-md px-3 py-2 text-sm font-medium tracking-heading ${className || ''}`
+ );
+ const classNames = twMerge(
+ `${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 ease-in-out rounded-md px-4 py-3 text-md font-semibold tracking-heading ${className || ''}`
+ );
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const smallButtonClasses = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 | |
ease-in-out rounded-md px-3 py-2 text-sm font-medium tracking-heading ${className || ''}`); | |
const classNames = twMerge(`${bgClassName} ${textClassName} transition-all duration-500 ease-in-out | |
const classNames = twMerge(`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 ease-in-out | |
rounded-md px-4 py-3 text-md font-semibold tracking-heading ${className || ''}`); | |
const smallButtonClasses = twMerge( | |
`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 ease-in-out rounded-md px-3 py-2 text-sm font-medium tracking-heading ${className || ''}` | |
); | |
const classNames = twMerge( | |
`${bgClassName} ${textClassName} border-primary-500 border transition-all duration-500 ease-in-out rounded-md px-4 py-3 text-md font-semibold tracking-heading ${className || ''}` | |
); |
🧰 Tools
🪛 ESLint
[error] 65-65: Insert ⏎···
(prettier/prettier)
[error] 67-67: Insert ⏎···
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 65-67: Insert ⏎···
prettier/prettier
We don't need such type of animations for buttons. |
Description
ScreenRecording.zip
Summary by CodeRabbit