Skip to content

Conversation

@87fmartin
Copy link

@aniatat
Copy link

aniatat commented Jun 30, 2025

@Conductor-Codes review

Comment on lines +54 to +58
<button
onClick={() => removeFromCart(idx)}
style={{marginLeft: 8}}>
Remove
</button>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<button
onClick={() => removeFromCart(idx)}
style={{marginLeft: 8}}>
Remove
</button>
<button
onClick={() => removeFromCart(idx)}
style={{marginLeft: 8}}
aria-label={`Remove ${item.name} from cart`}>
Remove
</button>

The "Remove" button lacks accessible context for screen reader users. Without additional context, screen reader users won't know which item the button will remove, making the interface confusing and potentially causing unintentional actions.

Review by Conductor

Is this review helpful? React 👍 or 👎 to let us know!

Comment on lines +71 to +73
<button onClick={() => addToCart(product)} style={{marginTop: 8}}>
Add to Cart
</button>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<button onClick={() => addToCart(product)} style={{marginTop: 8}}>
Add to Cart
</button>
<button
onClick={() => addToCart(product)}
style={{marginTop: 8}}
aria-label={`Add ${product.name} to Cart`}>
Add to Cart
</button>

The "Add to Cart" button lacks an accessible name that includes the product being added. Screen reader users won't know which product they're adding to the cart, as the button text is the same for all products.

Review by Conductor

Is this review helpful? React 👍 or 👎 to let us know!

Comment on lines +22 to +32
<Link href="/Store" style={{float: 'right', marginBottom: 16}}>
Back to Store
</Link>
{cart.length === 0 ? (
<p>Your cart is empty.</p>
) : (
<ul style={{listStyle: 'none', padding: 0}}>
{cart.map((item, idx) => (
<li
key={idx}
style={{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Link href="/Store" style={{float: 'right', marginBottom: 16}}>
Back to Store
</Link>
{cart.length === 0 ? (
<p>Your cart is empty.</p>
) : (
<ul style={{listStyle: 'none', padding: 0}}>
{cart.map((item, idx) => (
<li
key={idx}
style={{
<style jsx global>{`
a:focus, button:focus {
outline: 2px solid #007bff;
outline-offset: 2px;
}
`}</style>
<Link href="/Store" style={{float: 'right', marginBottom: 16}}>
Back to Store
</Link>
{cart.length === 0 ? (
<p>Your cart is empty.</p>
) : (

All interactive elements (buttons and links) lack visible focus indicators. Without proper focus styles, keyboard users won't be able to see which element is currently focused when navigating, making the interface inaccessible for keyboard-only users.

Review by Conductor

Is this review helpful? React 👍 or 👎 to let us know!

Comment on lines +45 to +53
<Link href="/Cart" style={{float: 'right', marginBottom: 16}}>
Go to Cart ({cart.length})
</Link>
<div style={{display: 'flex', gap: 32, flexWrap: 'wrap'}}>
{products.map((product) => (
<div
key={product.id}
style={{
border: '1px solid #eee',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Link href="/Cart" style={{float: 'right', marginBottom: 16}}>
Go to Cart ({cart.length})
</Link>
<div style={{display: 'flex', gap: 32, flexWrap: 'wrap'}}>
{products.map((product) => (
<div
key={product.id}
style={{
border: '1px solid #eee',
<Link href="/Cart" className="focusable-element" style={{float: 'right', marginBottom: 16}}>
Go to Cart ({cart.length})
</Link>
<div style={{display: 'flex', gap: 32, flexWrap: 'wrap'}}>
{products.map((product) => (
<div
key={product.id}
style={{
border: '1px solid #eee',
borderRadius: 8,
padding: 16,
width: 220,
}}>

All interactive elements (buttons and links) lack visible focus indicators. This makes it impossible for keyboard users to see which element is currently focused, creating a significant accessibility barrier for people who cannot use a mouse.

Review by Conductor

Is this review helpful? React 👍 or 👎 to let us know!

Comment on lines +45 to +53
<Link href="/Cart" style={{float: 'right', marginBottom: 16}}>
Go to Cart ({cart.length})
</Link>
<div style={{display: 'flex', gap: 32, flexWrap: 'wrap'}}>
{products.map((product) => (
<div
key={product.id}
style={{
border: '1px solid #eee',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Link href="/Cart" style={{float: 'right', marginBottom: 16}}>
Go to Cart ({cart.length})
</Link>
<div style={{display: 'flex', gap: 32, flexWrap: 'wrap'}}>
{products.map((product) => (
<div
key={product.id}
style={{
border: '1px solid #eee',
<Link href="/Cart" className="focusable-element" style={{float: 'right', marginBottom: 16}}>
Go to Cart ({cart.length})
</Link>
<div style={{display: 'flex', gap: 32, flexWrap: 'wrap'}}>
{products.map((product) => (
<div
key={product.id}
style={{
border: '1px solid #eee',
borderRadius: 8,
padding: 16,
width: 220,
}}>

All interactive elements (buttons and links) lack visible focus indicators. This makes it impossible for keyboard users to see which element is currently focused, creating a significant accessibility barrier for people who cannot use a mouse.

Review by Conductor

Is this review helpful? React 👍 or 👎 to let us know!

Comment on lines +45 to +53
<Link href="/Cart" style={{float: 'right', marginBottom: 16}}>
Go to Cart ({cart.length})
</Link>
<div style={{display: 'flex', gap: 32, flexWrap: 'wrap'}}>
{products.map((product) => (
<div
key={product.id}
style={{
border: '1px solid #eee',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Link href="/Cart" style={{float: 'right', marginBottom: 16}}>
Go to Cart ({cart.length})
</Link>
<div style={{display: 'flex', gap: 32, flexWrap: 'wrap'}}>
{products.map((product) => (
<div
key={product.id}
style={{
border: '1px solid #eee',
<Link href="/Cart" style={{float: 'right', marginBottom: 16}} className="no-bg-scrollbar" onFocus={(e) => e.target.style.outline = '2px solid #4299e1'} onBlur={(e) => e.target.style.outline = 'none'}>
Go to Cart ({cart.length})
</Link>
<div style={{display: 'flex', gap: 32, flexWrap: 'wrap'}}>
{products.map((product) => (
<div
key={product.id}
style={{
border: '1px solid #eee',
borderRadius: 8,
padding: 16,
width: 220,
}}>

All interactive elements (buttons and links) lack visible focus indicators. This makes it impossible for keyboard users to see which element is currently focused, creating a significant accessibility barrier for people who cannot use a mouse.

Review by Conductor

Is this review helpful? React 👍 or 👎 to let us know!

Comment on lines +58 to +67
<img
src={product.image}
alt={product.name}
style={{
width: '100%',
height: 140,
objectFit: 'cover',
borderRadius: 4,
}}
/>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<img
src={product.image}
alt={product.name}
style={{
width: '100%',
height: 140,
objectFit: 'cover',
borderRadius: 4,
}}
/>
<img
src={product.image}
alt={product.name}
width="220"
height="140"
style={{
width: '100%',
height: 140,
objectFit: 'cover',
borderRadius: 4,
}}
/>

Images without explicit width and height HTML attributes can cause layout shifts as the browser doesn't know the image dimensions until they load. This negatively impacts Cumulative Layout Shift (CLS), an important Core Web Vitals metric.

Review by Conductor

Is this review helpful? React 👍 or 👎 to let us know!

Comment on lines +39 to +49
<img
src={item.image}
alt={item.name}
style={{
width: 48,
height: 48,
objectFit: 'cover',
borderRadius: 4,
marginRight: 16,
}}
/>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<img
src={item.image}
alt={item.name}
style={{
width: 48,
height: 48,
objectFit: 'cover',
borderRadius: 4,
marginRight: 16,
}}
/>
<img
src={item.image}
alt={item.name}
width={48}
height={48}
style={{
width: 48,
height: 48,
objectFit: 'cover',
borderRadius: 4,
marginRight: 16,
}}
/>

Images without explicit width and height HTML attributes can cause layout shifts as the browser doesn't know the image dimensions until they load. This negatively impacts Cumulative Layout Shift (CLS), an important Core Web Vitals metric.

Review by Conductor

Is this review helpful? React 👍 or 👎 to let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants