Update course page | Clean up grades page - #344
Open
neron-png wants to merge 13 commits into
Open
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| /** Coefficients are whole numbers of at most two digits. */ | ||
| export function normalizeCoefficient(coefficient: number | string): string { | ||
| if (!coefficient) return String(coefficient ?? ''); |
|
|
||
| export function gradeDistributionChart(canvas: HTMLCanvasElement, statistics: ExamStatistics) { | ||
| const barColorPassed = "#004C96"; | ||
| const barColorFailed = "#A2A7AF" |
Member
Author
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes dead code, gives components consistent PascalCase names that say
what they render, and lifts shared types and chart config out of the
components. No UI changes.
Structure:
- Delete newGrades.svelte (empty), flipstore.ts and getSemester.ts (both
verbatim duplicates of helpers.ts), and grades.svelte (a 24-line {#each}
indirection, now inlined into GradesPage).
- helpers.ts becomes the shared home for getSemester + the flipped store.
- Chart config moves to grades/charts.ts as a Svelte action, mirroring
courses/charts.ts. StatsCard loses its afterUpdate/bind:this/manual
destroy, and no longer rebuilds the whole Chart on every update.
Kept separate from helpers.ts: chart.js is ~200KB behind a side-effecting
import, so folding it in would drag it into every helpers consumer.
- SemesterGroup/Registration/AveragesResult were declared in three files;
they now live in $types/grades.
- Semester state is passed as plain arrays rather than Writable props.
Fixes:
- Custom course ids restarted from 0 on reload while the courses persisted
in localStorage, so a new course could collide with a restored one and
break both deletion and the getElementById lookups. nextId is now seeded
past the highest persisted id, and the {#each} is keyed.
- Flipper created a new store subscription on every reactive re-run and
never unsubscribed. Uses auto-subscription now.
- inputUpdate assigned to `course` 13 lines before its `var` declaration,
working only via hoisting. Uses a local const.
- The degree calculator's semester label was hardcoded Greek; it now uses
getSemester + $t like the grades side.
Also drops unused imports: leaflet/cheerio in the degree card (bad
auto-imports; tree-shaken, so no size change) and an unrendered Card in
recents.svelte.
chart.js ships unregistered and throws at runtime when a chart's controller was never registered. Only statsCard imported 'chart.js/auto', so its side effect silently supplied registration for courses/charts.ts, which imports the bare 'chart.js' and registers nothing itself. Nothing in the source guaranteed that ordering. coursePage is reachable from the homepage recents via recentGrades.svelte's navController.push, with no grades page in the graph, so the course page charts only render because rollup happened to merge the registration, courses/charts.ts and averages into one chunk that the homepage pulls in for its GPA display. A different chunking decision would have broken them. Both chart modules now import Chart from $lib/chart, which owns the registration, so each chart carries its own rather than depending on an unrelated feature having been loaded first.
The calculator read its state back out of the DOM: inputUpdate walked document.getElementById(course.id) for every row, parsed .value, and wrote validation styling straight onto .style.borderColor. main/fetchData/ inputUpdate/numberCheck/checkPrecision/degreeGradeUpdate/gradeString each mutated objects handed in by the caller, which is why the card needed `degree_grade = degree_grade` three times to make svelte notice. Those seven files collapse into two: degreeGrade.ts (pure: input masking, the averages, formatting) and fetchDegreeData.ts (the I/O). The card now derives the grade reactively from state — `$: degreeGrade = computeDegreeGrade(passedSums, [...unpassedCourses, ...$customCourses])` — and inputs use bind:value with class:invalid instead of getElementById and inline styles. CustomCourseRow takes named handlers rather than the drilled clickInput/gradeInput pair. The masking is unusual and load-bearing, so it was pinned down first: a temporary differential test ran the original numberCheck/checkPrecision against the replacement over 136 raw/previous combinations, all matching. degreeGrade.test.ts keeps those expectations, quirks included (85 -> 8.5, 80 -> 8.0, 10.5 -> 1.05, 00 -> 0.0, >= 100 rejected). Behaviour this changes, all previously broken: - Custom course edits now persist. bind:value mutated the store's objects in place without ever calling set, so nothing was written to localStorage until an add or delete happened to trigger it. - Restored custom courses now count towards the average on load. inputUpdate only ran on an input event, so saved predictions were ignored until you typed somewhere. - Input borders no longer reset to a hardcoded #ccc on every keystroke, which in dark mode flipped them from the themed colour to light grey. The invalid state is a class now, so the CSS variable holds. - The standalone /degreeCalculator route rendered <Card /> without the required flip prop; it now defaults to a no-op.
Its logic lived under lib/functions/degreeCalculator while its components
lived under lib/components/degreeCalculator, so the feature spanned two
trees. Nothing outside the feature imported the logic, which is what
separates it from gradeAverages — that one is genuinely shared with the
homepage and stays put. Moving it alongside the components matches how
courses/ already keeps charts.ts next to coursePage.svelte.
CoursesSkeleton was 13 lines used only as the card's {#await} pending
branch, so it moves inline. That also retires a second component named
CoursesSkeleton, which collided with the unrelated one in courses/.
degreeGrade.ts and fetchDegreeData.ts stay separate: degreeGrade.ts imports
nothing, which is why its tests need no mocking, and fetchDegreeData pulls
in the whole dataService chain.
Flipper is a shared component, but it imported courseAdded from the degree calculator: the calculator bumped a counter on add so the flipper knew to re-measure its height. A generic component reaching into one feature for a signal is the wrong direction, and the counter only covered adds — deleting a course never re-measured, and the bump ran during the reactive cycle, so the height could be read before the DOM had the new row. A ResizeObserver on the two faces covers every cause and runs after layout, so courseAdded goes away entirely along with the dependency. Height writes are now guarded on a real change, so the observer can't re-enter.
Pushing the course page over grades makes ionic put .ion-page-hidden (display: none) on the grades page, so both flip faces measure 0. The ResizeObserver added in 37f26c6 fired on that and wrote height: 0px, collapsing the card. With the content that much shorter the scroll position underneath got clamped, so popping back landed the page at a different scroll offset than it was left at. Skip the write when the measurement is 0 and keep the last real height, so a covered page reappears exactly as it was left.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.