Display semester half hours as Q1/Q2 or Q3/Q4 based on semester#174
Display semester half hours as Q1/Q2 or Q3/Q4 based on semester#174
Conversation
cjquines
left a comment
cjquines
left a comment
There was a problem hiding this comment.
can you screenshot what this looks like? i forget how much space there is there in that area
| // Use Q1/Q2 for fall semester, Q3/Q4 for spring semester | ||
| const [firstLabel, secondLabel] = state.term.semester === 'f' ? ['Q1', 'Q2'] : ['Q3', 'Q4']; | ||
|
|
||
| return `\u2264 ${hours.toFixed(1)} hours (${hoursFirstHalf.toFixed(1)} ${firstLabel}, ${hoursSecondHalf.toFixed(1)} ${secondLabel})`; |
There was a problem hiding this comment.
we might as well use ≤ directly in the string
also, what's the rationale for having ≤ here?
There was a problem hiding this comment.
oh i see the rationale for ≤
can we have a tooltip instead though?
| hours: number; | ||
| hoursFirstHalf: number; | ||
| hoursSecondHalf: number; |
There was a problem hiding this comment.
what do you think about the type
hours: { firstHalf: number; secondHalf: number; }There was a problem hiding this comment.
and omit the max hours, which i think we can recompute in the thing when we need to? unless we use it somewhere else
| if ('rawClass' in act) { | ||
| // Include if class is full semester or first half | ||
| return !act.rawClass.half || act.rawClass.half === 1; | ||
| } | ||
| // Include all non-class activities in both halves | ||
| return true; |
There was a problem hiding this comment.
what do you feel about
act => !('rawClass' in act) || !act.rawClass.half || act.rawClass.half === 1or perhaps
act => [
!('rawClass' in act), // non-class activity
!act.rawClass.half, // full-semester class
act.rawClass.half === 1, // first half
].some(Boolean)There was a problem hiding this comment.
this is very nit-ty btw, so take it or leave it
|
also thanks for the pr! |
|
Here is a screenshot: I'll look over your other comments soon. On a cursory glance I agree with all of them. |
|
hey @HPoincare sorry for the late ping 😓 do you plan to address the comments here or |
2 participants
Addresses issue #147