-
Notifications
You must be signed in to change notification settings - Fork 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
Merge course popovers #551
Open
CadenLee2
wants to merge
19
commits into
main
Choose a base branch
from
merge-course-popovers
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+234
−71
Open
Changes from 13 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
129bbbe
feat: merge warning into course popover
Cadecraft 82498db
feat: replace info icon with warning icon if issue
CadenLee2 a2dd379
feat: make popover styles conform more to figma
CadenLee2 9cbaa93
feat: increase popover arrow size
CadenLee2 a5b451e
fix: correct popover directions with arrows
CadenLee2 e194666
fix: popover sizing details
CadenLee2 ae136a1
Merge branch 'main' of https://github.com/icssc/peterportal-client in…
CadenLee2 101a0ce
feat: popover arrow shadow
CadenLee2 07aeee9
refactor: move course popover to new component
CadenLee2 c323679
fix: course popover line heights
CadenLee2 de9fa61
fix: course popover props interface
CadenLee2 e0e2c05
fix: popover position on mobile
CadenLee2 025aa9e
Merge branch 'main' of https://github.com/icssc/peterportal-client in…
CadenLee2 98bb1dd
refactor: remove separate popover position classes
CadenLee2 77a1e70
Merge branch 'main' into merge-course-popovers
CadenLee2 92d84bb
Adjust styles of tooltip
Awesome-E 4ec9a23
fix tooltips in other directions
Awesome-E ffb01fa
update from main
CadenLee2 4584700
fix: bottom popover arrow color
CadenLee2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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 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,62 @@ | ||
.course-popover { | ||
line-height: normal; | ||
|
||
.popover-name { | ||
font-weight: bold; | ||
color: var(--text-dark); | ||
font-size: 16px; | ||
height: 21px; | ||
margin-top: -2px; | ||
} | ||
|
||
.popover-subtitle { | ||
font-weight: bold; | ||
} | ||
|
||
.popover-units { | ||
font-weight: normal; | ||
} | ||
|
||
.popover-description { | ||
font-size: 14px; | ||
padding-top: 12px; | ||
line-height: 17px; | ||
} | ||
|
||
.popover-detail { | ||
padding-top: 12px; | ||
} | ||
|
||
.popover-detail-prefix { | ||
font-weight: bold; | ||
} | ||
|
||
.popover-detail-warning { | ||
font-size: 14px; | ||
color: #ce0000; | ||
gap: 5px; | ||
|
||
.popover-detail-warning-icon { | ||
margin-bottom: 4px; | ||
margin-right: 4px; | ||
font-size: 16px; | ||
} | ||
} | ||
|
||
.popover-detail-italics { | ||
padding-top: 6px; | ||
font-weight: 400; | ||
font-size: 12px; | ||
font-style: italic; | ||
color: var(--petr-gray); | ||
} | ||
} | ||
|
||
[data-theme='dark'] { | ||
.popover-detail-warning { | ||
color: red; | ||
} | ||
.popover-detail-italics { | ||
color: var(--petr-gray); | ||
} | ||
} |
This file contains 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,68 @@ | ||
import { FC } from 'react'; | ||
import './CoursePopover.scss'; | ||
import { ExclamationTriangle } from 'react-bootstrap-icons'; | ||
import Popover from 'react-bootstrap/Popover'; | ||
|
||
interface CoursePopoverProps { | ||
department: string; | ||
courseNumber: string; | ||
title: string; | ||
minUnits: number; | ||
maxUnits: number; | ||
description: string; | ||
prerequisiteText: string; | ||
corequisites: string; | ||
requiredCourses?: string[]; | ||
} | ||
|
||
const CoursePopover: FC<CoursePopoverProps> = (props) => { | ||
const { | ||
department, | ||
courseNumber, | ||
title, | ||
minUnits, | ||
maxUnits, | ||
description, | ||
prerequisiteText, | ||
corequisites, | ||
requiredCourses, | ||
} = props; | ||
|
||
return ( | ||
<Popover.Content className="course-popover"> | ||
<div className="popover-name"> | ||
{department + ' ' + courseNumber + ' '} | ||
<span className="popover-units">({minUnits === maxUnits ? minUnits : `${minUnits}-${maxUnits}`} units)</span> | ||
</div> | ||
<div className="popover-description"> | ||
<span className="popover-subtitle">{title + ':'}</span> | ||
<br /> | ||
{description} | ||
</div> | ||
{prerequisiteText && ( | ||
<div className="popover-detail"> | ||
<span className="popover-detail-prefix">Prerequisites:</span> {prerequisiteText} | ||
</div> | ||
)} | ||
{corequisites && ( | ||
<div className="popover-detail"> | ||
<span className="popover-detail-prefix">Corequisites:</span> {corequisites} | ||
</div> | ||
)} | ||
{requiredCourses && ( | ||
<div className="popover-detail"> | ||
<div className="popover-detail-warning"> | ||
<ExclamationTriangle className="popover-detail-warning-icon" /> | ||
Prerequisite{requiredCourses?.length > 1 ? 's' : ''} Not Met: {requiredCourses?.join(', ')} | ||
</div> | ||
<div className="popover-detail-italics"> | ||
Already completed? Click "Transfer Credits" at the top of the roadmap viewer to add{' '} | ||
{requiredCourses?.length > 1 ? 'these prerequisites' : 'this prerequisite'}. | ||
</div> | ||
</div> | ||
)} | ||
</Popover.Content> | ||
); | ||
}; | ||
|
||
export default CoursePopover; |
This file contains 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 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 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 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 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
Oops, something went wrong.
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.
Based on research into the Boostrap
Popover
, it seems this is needed, but using!important
is typically not good practiceTried using
margin-top
, differentposition
s,transform: translate
(these do nothing)Tried:
(this does nothing)
Tried:
This results in partial success, but does not render the shadow or border correctly on the top portion (especially visible in light mode)--the shadow is vertically too short, so it would take multiple overlapping and offset shadows to cover that portion
Suggestions would be appreciated
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.
yeah it looks like it was styled on the element itself, so
!important
is the only way to override that