feat: draft UI for prerequisite summary #4401
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| @import '~styles/utils/modules-entry'; | ||
|
|
||
| .tooltip { | ||
| padding: 0.75rem 1rem; | ||
| } | ||
|
|
||
| .trigger { | ||
| display: inline-flex; | ||
| flex-shrink: 0; | ||
| justify-content: center; | ||
| align-items: center; | ||
| padding: 0.15rem; | ||
| border: 0; | ||
| line-height: 0; | ||
| color: var(--gray-light); | ||
| background: transparent; | ||
| cursor: pointer; | ||
|
|
||
| &:hover, | ||
| &:focus { | ||
| outline: none; | ||
| color: theme-color(); | ||
| } | ||
|
Contributor
There was a problem hiding this comment.
Prompt To Fix With AIThis is a comment left during a code review.
Path: website/src/views/components/module-info/RequisiteRulePopup.scss
Line: 19-23
Comment:
**Focus outline removed without replacement**
`outline: none` on `:focus` suppresses the browser's default focus ring for keyboard users without a visible alternative. The color shift to `theme-color()` on a small icon button is unlikely to meet WCAG 2.4.7 Focus Visible. Consider using `outline: none` only on `:focus:not(:focus-visible)` and keeping the ring for `:focus-visible`, or adding an explicit `box-shadow`/`outline` replacement to keep keyboard navigation accessible.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| :global(.svg) { | ||
| display: block; | ||
| width: 1.1rem; | ||
| height: 1.1rem; | ||
| } | ||
| } | ||
|
|
||
| .content { | ||
| max-width: 28rem; | ||
| font-size: $font-size-sm; | ||
| text-align: left; | ||
| white-space: pre-wrap; | ||
| word-break: break-word; | ||
| color: var(--tooltip-color); | ||
|
|
||
| :global(a) { | ||
| color: theme-color(); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { screen } from '@testing-library/react'; | ||
| import { Provider } from 'react-redux'; | ||
|
|
||
| import { initAction } from 'test-utils/redux'; | ||
| import reducers from 'reducers'; | ||
| import configureStore from 'bootstrapping/configure-store'; | ||
| import renderWithRouterMatch from 'test-utils/renderWithRouterMatch'; | ||
| import RequisiteRulePopup from './RequisiteRulePopup'; | ||
|
|
||
| describe(RequisiteRulePopup, () => { | ||
| function make(rule: string, summary?: string) { | ||
| const initialState = reducers(undefined, initAction()); | ||
| const { store } = configureStore(initialState); | ||
|
|
||
| return renderWithRouterMatch( | ||
| <Provider store={store}> | ||
| <RequisiteRulePopup rule={rule} summary={summary} /> | ||
| </Provider>, | ||
| {}, | ||
| ); | ||
| } | ||
|
|
||
| it('renders nothing when rule matches summary', () => { | ||
| const rule = 'CS1010 or its equivalent'; | ||
| const { view } = make(rule, rule); | ||
| expect(view.container).toBeEmptyDOMElement(); | ||
| }); | ||
|
|
||
| it('renders info icon with accessible label', () => { | ||
| make('PROGRAM_TYPES IF_IN Undergraduate Degree THEN (COURSES (1) CS1010:D)', 'CS1010'); | ||
|
|
||
| expect( | ||
| screen.getByRole('button', { name: 'View detailed prerequisite rule' }), | ||
| ).toBeInTheDocument(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import { Info } from 'react-feather'; | ||
|
|
||
| import LinkModuleCodes from 'views/components/LinkModuleCodes'; | ||
| import Tooltip from 'views/components/Tooltip'; | ||
| import styles from './RequisiteRulePopup.scss'; | ||
|
|
||
| type Props = { | ||
| rule: string; | ||
| summary?: string; | ||
| label?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Info icon that shows the full structured requisite rule in a popup. | ||
| */ | ||
| export default function RequisiteRulePopup({ | ||
| rule, | ||
| summary, | ||
| label = 'View detailed prerequisite rule', | ||
| }: Props) { | ||
| if (!rule.trim()) return null; | ||
| if (summary?.trim() === rule.trim()) return null; | ||
|
|
||
| const content = ( | ||
| <div className={styles.content}> | ||
| <LinkModuleCodes>{rule}</LinkModuleCodes> | ||
| </div> | ||
| ); | ||
|
|
||
| return ( | ||
| <Tooltip | ||
| content={content} | ||
| interactive | ||
| arrow | ||
| placement="bottom" | ||
| maxWidth="none" | ||
| touch="hold" | ||
| className={styles.tooltip} | ||
| > | ||
| <button type="button" className={styles.trigger} aria-label={label}> | ||
| <Info className="svg svg-small" size={18} strokeWidth={2} /> | ||
| </button> | ||
| </Tooltip> | ||
| ); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
outline: noneon:focusdiscards the browser's default focus ring for keyboard users. The icon color change alone (from--gray-lighttotheme-color()) is unlikely to meet WCAG 2.1 SC 2.4.11's visible focus requirement, and many custom themes may have low contrast between these two values. Consider replacing with a visible focus-visible outline (e.g.,outline: 2px solid currentColor) instead of suppressing it.Prompt To Fix With AI
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.
Fixed in 3a74cc1 — split hover/focus styles and added a
:focus-visibleoutline (2px solid currentColorwith offset) while keepingoutline: noneonly for:focus:not(:focus-visible).