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.
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.
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.