fix: restriction pop up on mobile #1400
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds mobile support for restriction code popovers in the RestrictionsCell component. On mobile devices where hover interactions aren't available, clicking a restriction code now opens a Popover that displays the restriction descriptions along with a "University Requirements" link.
Key Changes:
- Added mobile-specific interaction pattern using Popover instead of Tooltip
- Implemented toggle behavior for opening/closing the popover on mobile
- Maintained existing desktop tooltip behavior unchanged
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...omponents/RightPane/SectionTable/SectionTableBody/SectionTableBodyCells/RestrictionsCell.tsx
Outdated
Show resolved
Hide resolved
| <Typography | ||
| component="a" | ||
| href="https://www.reg.uci.edu/enrollment/restrict_codes.html" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| onClick={handleClick} | ||
| sx={{ cursor: 'pointer' }} | ||
| > | ||
| {restrictions} | ||
| </a> | ||
| </Typography> | ||
| </Tooltip> | ||
| </Typography> |
There was a problem hiding this comment.
The mobile link element has an href but prevents navigation with event.preventDefault(). This creates a confusing pattern - the link appears clickable but doesn't navigate. Consider either: 1) Removing the href and using role="button" with tabIndex={0} to properly indicate it's a button that opens a popover, or 2) Removing the preventDefault to allow navigation while the popover provides additional context before navigation.
There was a problem hiding this comment.
this is kinda troll that my review is just what the copilot said lol.
chore: seems like you're repurposing the anchor tag to create the styling for the button. It's clever but a little hacky even though it's a small snippet it's better practice to just mimic the styling with the css classes and make it a button. somn like that correct me if I'm wrong
...omponents/RightPane/SectionTable/SectionTableBody/SectionTableBodyCells/RestrictionsCell.tsx
Show resolved
Hide resolved
KevinWu098
requested review from
alexespejo
and removed request for
KevinWu098
|
Deferring review over to @alexespejo 🫡 |
|
/deploy |
...omponents/RightPane/SectionTable/SectionTableBody/SectionTableBodyCells/RestrictionsCell.tsx
Outdated
Show resolved
Hide resolved
| <Typography | ||
| component="a" | ||
| href="https://www.reg.uci.edu/enrollment/restrict_codes.html" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| onClick={handleClick} | ||
| sx={{ cursor: 'pointer' }} | ||
| > | ||
| {restrictions} | ||
| </a> | ||
| </Typography> | ||
| </Tooltip> | ||
| </Typography> |
There was a problem hiding this comment.
this is kinda troll that my review is just what the copilot said lol.
chore: seems like you're repurposing the anchor tag to create the styling for the button. It's clever but a little hacky even though it's a small snippet it's better practice to just mimic the styling with the css classes and make it a button. somn like that correct me if I'm wrong
...omponents/RightPane/SectionTable/SectionTableBody/SectionTableBodyCells/RestrictionsCell.tsx
Outdated
Show resolved
Hide resolved
AureliaSindhu
had a problem deploying
to
staging-1400
GitHub Actions
Failure
AureliaSindhu
had a problem deploying
to
staging-1400
GitHub Actions
Failure
AureliaSindhu
had a problem deploying
to
staging-1400
GitHub Actions
Failure
AureliaSindhu
had a problem deploying
to
staging-1400
GitHub Actions
Failure
AureliaSindhu
had a problem deploying
to
staging-1400
GitHub Actions
Failure
Summary
Added mobile support for restriction code popovers in RestrictionsCell. On mobile, clicking a restriction code opens a Popover with descriptions and a "University Requirements" link, since hover isn't available.
Test Plan
Use mobile view and click on restrictions for a chosen class.
Issues
Closes #1154