-
Notifications
You must be signed in to change notification settings - Fork 0
Address PR #7 review feedback: accessibility, CSS cleanup, and functionality fixes #14
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
Address PR #7 review feedback: accessibility, CSS cleanup, and functionality fixes #14
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add aria-labels to TopicChip buttons for screen reader support - Add consistent hover:bg-[position:100%_0] to Button danger/success variants - Fix SuggestedPrompts to open YouTube search in new tab - Add "Example Preview" label to video preview card and remove cursor-pointer - Fix navigation Link group-hover by adding inline-flex items-center gap-2 - Rename duplicate shimmer animation to skeleton-shimmer - Remove unused CSS utility classes (.input-glow, .chip, .text-shadow-*) - Add prefers-reduced-motion support with motion-reduce classes - Use CSS custom properties (--animation-delay) for animation delays Co-authored-by: groupthinking <154503486+groupthinking@users.noreply.github.com>
The rebrand to "Video2Learn" was incomplete - only the homepage was changed while all other pages (dashboard, playground), metadata, and API still use UVAI.io. Reverting to maintain consistent branding across the application. Co-authored-by: groupthinking <154503486+groupthinking@users.noreply.github.com>
- Remove CSS custom property wrapper for animation delays (use direct values) - Remove unused .animate-shimmer class that was never used in components - Simplify inline style objects in SuggestedPrompts Co-authored-by: groupthinking <154503486+groupthinking@users.noreply.github.com>
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.
Pull request overview
This pull request addresses review feedback from PR #7 by implementing accessibility improvements, fixing functionality issues, cleaning up CSS, and reverting incomplete branding changes.
Changes:
- Added accessibility features including
aria-labelattributes and comprehensivemotion-reducesupport for users with motion sensitivity preferences - Fixed SuggestedPrompts functionality to open YouTube searches in new tabs instead of breaking video processing by setting invalid URLs
- Cleaned up CSS by removing unused classes and resolving duplicate animation keyframe naming conflicts
- Reverted incomplete "Video2Learn" rebrand back to "UVAI.io" for consistency across the application
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/web/src/components/ui/SuggestedPrompts.tsx | Added aria-label to topic chips and implemented motion-reduce classes for accessibility; added style prop forwarding |
| apps/web/src/components/ui/Button.tsx | Added gradient position shift on hover to danger and success button variants for consistency with primary variant |
| apps/web/src/app/page.tsx | Fixed SuggestedPrompts to open YouTube search in new tab; clarified video preview as decorative example; reverted branding from "Video2Learn" to "UVAI.io"; improved navigation link hover behavior |
| apps/web/src/app/globals.css | Renamed duplicate shimmer keyframe to skeleton-shimmer to resolve conflict; removed unused CSS classes (.input-glow, .chip, .text-shadow-lg, .text-shadow-glow, .animate-shimmer) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
da47fd3
into
claude/polish-ui-design-S5CS6
| 'w-full max-w-2xl mx-auto', | ||
| 'animate-fade-in-up', | ||
| 'animate-fade-in-up motion-reduce:animate-none motion-reduce:opacity-100', | ||
| className | ||
| )} | ||
| style={{ animationDelay: '200ms', opacity: 0 }} | ||
| style={{ | ||
| ...style, | ||
| animationDelay: '200ms', | ||
| opacity: 0, | ||
| animationFillMode: 'forwards' | ||
| }} | ||
| {...props} |
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.
Bug: An inline style opacity: 0 overrides the motion-reduce:opacity-100 class, making the component invisible for users with reduced motion enabled.
Severity: HIGH
Suggested Fix
Remove the opacity: 0 from the inline style object. The initial state should be managed by the animation keyframes (animate-fade-in-up) to ensure it does not conflict with the motion-reduce utility classes. This allows the motion-reduce:opacity-100 class to correctly apply when needed.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/web/src/components/ui/SuggestedPrompts.tsx#L111-L127
Potential issue: The component sets an inline style `opacity: 0`, which has higher CSS
specificity than the `motion-reduce:opacity-100` utility class. For users with the
`prefers-reduced-motion` accessibility setting enabled, animations are correctly
disabled, but the element remains invisible because the inline style takes precedence.
This makes the `SuggestedPrompts` container and its child `TopicChip` components
unusable for this group of users, which is a significant accessibility issue.
Did we get this right? 👍 / 👎 to inform future reviews.
Addresses 11 review comments from PR #7 covering accessibility gaps, broken functionality, CSS conflicts, and incomplete branding changes.
Accessibility
aria-labelto TopicChip buttons (Search for ${topic.label} videos)motion-reduceclasses throughout forprefers-reduced-motionsupportFunctionality
videoUrlstate (which breaks video processing)cursor-pointergroup-hover: addinline-flex items-center gap-2for proper arrow translationCSS
shimmerkeyframe toskeleton-shimmer(conflicted with existing animation at line 208).input-glow,.chip,.text-shadow-lg,.text-shadow-glow,.animate-shimmerhover:bg-[position:100%_0]to Button danger/success variants (matches primary variant gradient shift)Branding
Before:
After:
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.