Include colors and theme when sharing timetable #3467
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@leslieyip02 is attempting to deploy a commit to a Personal Account owned by @nusmodifications on Vercel. @nusmodifications first needs to authorize it. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3467 +/- ##
==========================================
+ Coverage 53.29% 53.34% +0.04%
==========================================
Files 271 271
Lines 5850 5892 +42
Branches 1395 1406 +11
==========================================
+ Hits 3118 3143 +25
- Misses 2732 2749 +17
☔ View full report in Codecov by Sentry. |
leslieyip02
changed the title
website/src/utils/timetables.ts
Outdated
| } | ||
|
|
||
| // Extracts module configs from query string | ||
| export function deserializeTimetable(serialized: string): SemTimetableConfig { |
There was a problem hiding this comment.
It's best to rewrite this so that it parses from query string, then delegate to objects to extract individual parts instead of having each functions re-parse the query string.
| if (semester && importedColors) { | ||
| Object.entries(importedColors).forEach((colorMapping: [string, ColorIndex]) => { | ||
| const [moduleCode, colorIndex] = colorMapping; | ||
| dispatch(selectModuleColor(semester, moduleCode, colorIndex)); | ||
| }); | ||
| } | ||
| const importedThemeId = deserializeTimetableThemeId(location.search); | ||
| if (semester && importedThemeId) { | ||
| dispatch(selectTheme(importedThemeId)); | ||
| } |
There was a problem hiding this comment.
Note that imported timetable is temporary - the user has to confirm before the existing timetable is overwritten. This makes the imported module colour and theme take effect immediately when opening the URL, which is inconsistent with timetable behavior
There was a problem hiding this comment.
demo-v2.mp4
Is this better?
| serializedModule.split(QUERY_PARAM_SEP).forEach((param) => { | ||
| const [paramName, paramValue] = param.split(QUERY_PARAM_VALUE_SEP); | ||
| if (paramName === QUERY_PARAM_ABBREV[COLOR_PARAM]) { | ||
| colorMapping[moduleCode] = parseInt(paramValue, 10); |
There was a problem hiding this comment.
Since colours are bound to a sub-param inside each module code, this is better extracted as part of deserializeTimetable instead
|
I wanted to implement undoing colors and theme imports, but it seems that the "UNDO" button in the notifications isn't working. I have opened a new issue for that: #3475. |
The issue has been resolved. Imported colors can be undone, but the theme does not become undone. I'm not sure why the theme is not stored in the history. I'm guessing it has something to do with this: |
Context
Resolves #3423 by allowing module colors and theme to be shared via URL.
Implementation
Module
colorMappingandthemeIdare passed into the query parameters.e.g.
demo.mp4