From f77edcf9aa825cda7557acd82a32970b8df67967 Mon Sep 17 00:00:00 2001 From: joeng03 Date: Thu, 3 Aug 2023 11:53:34 +0800 Subject: [PATCH 1/8] feat: Enable flexible number of colors for themes - instead of fixing the number of colors for a theme to 8 colors, now we are able to support a variable number of colors - add a new theme, 'Tequila' with 10 colors as Proof of Concept --- website/src/actions/theme.ts | 3 +- website/src/actions/timetables.ts | 11 ++++- website/src/data/themes.json | 41 ++++++++++++----- website/src/entry/export/TimetableOnly.tsx | 6 +-- website/src/reducers/theme.ts | 10 +++-- website/src/reducers/timetables.test.ts | 5 +++ website/src/reducers/timetables.ts | 2 +- website/src/styles/constants.scss | 12 +++++ website/src/types/reducers.ts | 1 + website/src/types/settings.ts | 1 + website/src/utils/colors.test.ts | 45 +++++++++++++------ website/src/utils/colors.ts | 25 ++++++----- website/src/views/components/ColorPicker.tsx | 7 ++- .../module-info/LessonTimetable.tsx | 5 ++- .../src/views/settings/SettingsContainer.tsx | 5 ++- website/src/views/settings/ThemeOption.scss | 7 +-- website/src/views/settings/ThemeOption.tsx | 17 ++++--- 17 files changed, 139 insertions(+), 64 deletions(-) diff --git a/website/src/actions/theme.ts b/website/src/actions/theme.ts index 3e9a311575..50e97cc80a 100644 --- a/website/src/actions/theme.ts +++ b/website/src/actions/theme.ts @@ -1,5 +1,6 @@ +import { Theme } from 'types/settings'; export const SELECT_THEME = 'SELECT_THEME' as const; -export function selectTheme(theme: string) { +export function selectTheme(theme: Theme) { return { type: SELECT_THEME, payload: theme, diff --git a/website/src/actions/timetables.ts b/website/src/actions/timetables.ts index 3378567c52..9e862bfc49 100644 --- a/website/src/actions/timetables.ts +++ b/website/src/actions/timetables.ts @@ -30,13 +30,19 @@ export const Internal = { }; }, - addModule(semester: Semester, moduleCode: ModuleCode, moduleLessonConfig: ModuleLessonConfig) { + addModule( + semester: Semester, + moduleCode: ModuleCode, + moduleLessonConfig: ModuleLessonConfig, + numOfColors: number, + ) { return { type: ADD_MODULE, payload: { semester, moduleCode, moduleLessonConfig, + numOfColors, }, }; }, @@ -64,8 +70,9 @@ export function addModule(semester: Semester, moduleCode: ModuleCode) { const lessons = getModuleTimetable(module, semester); const moduleLessonConfig = randomModuleLessonConfig(lessons); + const numOfColors = getState().theme.numOfColors; - dispatch(Internal.addModule(semester, moduleCode, moduleLessonConfig)); + dispatch(Internal.addModule(semester, moduleCode, moduleLessonConfig, numOfColors)); }); } diff --git a/website/src/data/themes.json b/website/src/data/themes.json index b3f82ec90d..38dc21a8da 100644 --- a/website/src/data/themes.json +++ b/website/src/data/themes.json @@ -1,50 +1,67 @@ [ { "id": "ashes", - "name": " Ashes" + "name": " Ashes", + "numOfColors": 8 }, { "id": "chalk", - "name": "Chalk" + "name": "Chalk", + "numOfColors": 8 }, { "id": "eighties", - "name": "Eighties" + "name": "Eighties", + "numOfColors": 8 }, { "id": "google", - "name": "Google" + "name": "Google", + "numOfColors": 8 }, { "id": "mocha", - "name": "Mocha" + "name": "Mocha", + "numOfColors": 8 }, { "id": "monokai", - "name": "Monokai" + "name": "Monokai", + "numOfColors": 8 }, { "id": "ocean", - "name": "Ocean" + "name": "Ocean", + "numOfColors": 8 }, { "id": "oceanic-next", - "name": "Oceanic Next" + "name": "Oceanic Next", + "numOfColors": 8 }, { "id": "paraiso", - "name": "Paraiso" + "name": "Paraiso", + "numOfColors": 8 }, { "id": "railscasts", - "name": "Railscasts" + "name": "Railscasts", + "numOfColors": 8 + }, + { + "id": "tequila", + "name": "Tequila", + "numOfColors": 10 }, { "id": "tomorrow", - "name": "Tomorrow" + "name": "Tomorrow", + "numOfColors": 8 }, { "id": "twilight", - "name": "Twilight" + "name": "Twilight", + "numOfColors": 8 } ] diff --git a/website/src/entry/export/TimetableOnly.tsx b/website/src/entry/export/TimetableOnly.tsx index 03f8dcfca6..89faa7574a 100644 --- a/website/src/entry/export/TimetableOnly.tsx +++ b/website/src/entry/export/TimetableOnly.tsx @@ -29,15 +29,15 @@ export default class TimetableOnly extends Component { render() { const { store } = this.props; - const theme = store.getState().theme.id; + const theme = store.getState().theme; const { semester, timetable, colors } = this.state; - const timetableColors = fillColorMapping(timetable, colors); + const timetableColors = fillColorMapping(timetable, colors, theme.numOfColors); return ( -
+
obj.id); function theme(state: ThemeState = defaultThemeState, action: Actions): ThemeState { - function setTheme(newTheme: string): ThemeState { + function setTheme(newTheme: Theme): ThemeState { // Update theme analytics info - withTracker((tracker) => tracker.setCustomDimension(DIMENSIONS.theme, newTheme)); + withTracker((tracker) => tracker.setCustomDimension(DIMENSIONS.theme, newTheme.id)); return { ...state, - id: newTheme, + id: newTheme.id, + numOfColors: newTheme.numOfColors, }; } @@ -39,7 +41,7 @@ function theme(state: ThemeState = defaultThemeState, action: Actions): ThemeSta const newThemeIndex = (themeIds.indexOf(state.id) + themeIds.length + action.payload) % themeIds.length; - return setTheme(themeIds[newThemeIndex]); + return setTheme(themes[newThemeIndex]); } case TOGGLE_TIMETABLE_ORIENTATION: return { diff --git a/website/src/reducers/timetables.test.ts b/website/src/reducers/timetables.test.ts index 57459dd2e7..f74239f40e 100644 --- a/website/src/reducers/timetables.test.ts +++ b/website/src/reducers/timetables.test.ts @@ -11,6 +11,9 @@ import { TimetablesState } from 'types/reducers'; import config from 'config'; import { PersistConfig } from 'redux-persist/es/types'; +// use 8 different colors for testing +const NUM_DIFFERENT_COLORS = 8; + const initialState = defaultTimetableState; jest.mock('config'); @@ -25,6 +28,7 @@ describe('color reducers', () => { semester: 1, moduleCode: 'CS1010S', moduleLessonConfig: {}, + numOfColors: NUM_DIFFERENT_COLORS, }, }).colors, ).toHaveProperty('1.CS1010S'); @@ -36,6 +40,7 @@ describe('color reducers', () => { semester: 2, moduleCode: 'CS3216', moduleLessonConfig: {}, + numOfColors: NUM_DIFFERENT_COLORS, }, }).colors, ).toHaveProperty('2.CS3216'); diff --git a/website/src/reducers/timetables.ts b/website/src/reducers/timetables.ts index fbb56e00c1..fbdf606558 100644 --- a/website/src/reducers/timetables.ts +++ b/website/src/reducers/timetables.ts @@ -131,7 +131,7 @@ function semColors(state: ColorMapping = defaultSemColorMap, action: Actions): C case ADD_MODULE: return { ...state, - [moduleCode]: getNewColor(values(state)), + [moduleCode]: getNewColor(values(state), action.payload.numOfColors), }; case REMOVE_MODULE: diff --git a/website/src/styles/constants.scss b/website/src/styles/constants.scss index b7b2dbcc1e..a1ec5b9cdf 100644 --- a/website/src/styles/constants.scss +++ b/website/src/styles/constants.scss @@ -145,6 +145,18 @@ $nusmods-theme-colors: ( #b6b3eb, #bc9458 ), + tequila: ( + #e7553e, + #ff9f1e, + #ffc7c7, + #f1baa1, + #ffd700, + #8febdb, + #a9daf5, + #80b9f3, + #b9848c, + #b249f2, + ), tomorrow: ( #c66, #de935f, diff --git a/website/src/types/reducers.ts b/website/src/types/reducers.ts index cd37df9e8f..118fbbd811 100644 --- a/website/src/types/reducers.ts +++ b/website/src/types/reducers.ts @@ -79,6 +79,7 @@ export const HORIZONTAL: TimetableOrientation = 'HORIZONTAL'; export type ThemeState = Readonly<{ id: string; + numOfColors: number; timetableOrientation: TimetableOrientation; showTitle: boolean; }>; diff --git a/website/src/types/settings.ts b/website/src/types/settings.ts index 3de27e2448..41a1459efe 100644 --- a/website/src/types/settings.ts +++ b/website/src/types/settings.ts @@ -3,6 +3,7 @@ export type ThemeId = string; export type Theme = { readonly id: ThemeId; readonly name: string; + readonly numOfColors: number; }; export type Mode = 'LIGHT' | 'DARK'; diff --git a/website/src/utils/colors.test.ts b/website/src/utils/colors.test.ts index 6ad2c01488..3af472542a 100644 --- a/website/src/utils/colors.test.ts +++ b/website/src/utils/colors.test.ts @@ -3,18 +3,23 @@ import { range, uniq, without } from 'lodash'; import { ColorMapping } from 'types/reducers'; import { ColorIndex, Lesson, SemTimetableConfig } from 'types/timetables'; -import { colorLessonsByKey, fillColorMapping, getNewColor, NUM_DIFFERENT_COLORS } from './colors'; +import { colorLessonsByKey, fillColorMapping, getNewColor } from './colors'; + +// use 8 different colors for testing +const NUM_DIFFERENT_COLORS = 8; describe(getNewColor, () => { test('it should get color without randomization', () => { // When there are no current colors - expect(getNewColor([], false)).toBe(0); + expect(getNewColor([], NUM_DIFFERENT_COLORS, false)).toBe(0); // When there are colors that have not been picked - expect(getNewColor([0, 1], false)).toBe(2); + expect(getNewColor([0, 1], NUM_DIFFERENT_COLORS, false)).toBe(2); // When all the colors have been picked once - expect(getNewColor(range(NUM_DIFFERENT_COLORS), false)).toBe(0); + expect(getNewColor(range(NUM_DIFFERENT_COLORS), NUM_DIFFERENT_COLORS, false)).toBe(0); // When all the colors have been picked once or more - expect(getNewColor([...range(NUM_DIFFERENT_COLORS), 0, 1], false)).toBe(2); + expect(getNewColor([...range(NUM_DIFFERENT_COLORS), 0, 1], NUM_DIFFERENT_COLORS, false)).toBe( + 2, + ); }); test('it should get random color', () => { @@ -23,7 +28,7 @@ describe(getNewColor, () => { // in [0, NUM_DIFFERENT_COLORS] AND not in unexpectedColors function expectValidIndex(unexpectedColors: ColorIndex[], currentColors: ColorIndex[]) { expect(without(range(NUM_DIFFERENT_COLORS), ...unexpectedColors)).toContain( - getNewColor(currentColors, true), + getNewColor(currentColors, NUM_DIFFERENT_COLORS, true), ); } @@ -61,7 +66,7 @@ describe(colorLessonsByKey, () => { lessons.push(newLesson); }); - const coloredLessons = colorLessonsByKey(lessons, 'venue'); + const coloredLessons = colorLessonsByKey(lessons, 'venue', NUM_DIFFERENT_COLORS); range(NUM_DIFFERENT_COLORS * 2).forEach((i) => { const coloredLesson = coloredLessons[i]; @@ -75,12 +80,17 @@ describe(colorLessonsByKey, () => { describe(fillColorMapping, () => { test('should return color map with colors for all modules', () => { - expect(Object.keys(fillColorMapping({ CS1010S: {}, CS3216: {} }, {}))).toEqual([ - 'CS1010S', - 'CS3216', - ]); + expect( + Object.keys(fillColorMapping({ CS1010S: {}, CS3216: {} }, {}, NUM_DIFFERENT_COLORS)), + ).toEqual(['CS1010S', 'CS3216']); - expect(fillColorMapping({ CS1010S: {}, CS3216: {} }, { CS1010S: 0, CS3216: 1 })).toEqual({ + expect( + fillColorMapping( + { CS1010S: {}, CS3216: {} }, + { CS1010S: 0, CS3216: 1 }, + NUM_DIFFERENT_COLORS, + ), + ).toEqual({ CS1010S: 0, CS3216: 1, }); @@ -89,13 +99,20 @@ describe(fillColorMapping, () => { fillColorMapping( { CS1010S: {}, CS3216: {} }, { CS1010S: 0, CS3216: 1, CS1101S: 1, CS2105: 0, CS1231: 2 }, + NUM_DIFFERENT_COLORS, ), ).toEqual({ CS1010S: 0, CS3216: 1, }); - expect(fillColorMapping({ CS1010S: {}, CS3216: {} }, { CS1010S: 0, CS3216: 0 })).toEqual({ + expect( + fillColorMapping( + { CS1010S: {}, CS3216: {} }, + { CS1010S: 0, CS3216: 0 }, + NUM_DIFFERENT_COLORS, + ), + ).toEqual({ CS1010S: 0, CS3216: 0, }); @@ -114,7 +131,7 @@ describe(fillColorMapping, () => { }; const uniqueColors = (timetable: SemTimetableConfig, colors: ColorMapping) => - uniq(Object.values(fillColorMapping(timetable, colors))); + uniq(Object.values(fillColorMapping(timetable, colors, NUM_DIFFERENT_COLORS))); expect(uniqueColors(FILLED_TIMETABLE, {})).toHaveLength(8); expect(uniqueColors(FILLED_TIMETABLE, { CS3216: 1, CS1101S: 0 })).toHaveLength(8); diff --git a/website/src/utils/colors.ts b/website/src/utils/colors.ts index 62bd0ba016..ae8b645e8f 100644 --- a/website/src/utils/colors.ts +++ b/website/src/utils/colors.ts @@ -4,21 +4,24 @@ import { ColorIndex, SemTimetableConfig } from 'types/timetables'; import { ColorMapping } from 'types/reducers'; import { ModuleCode } from 'types/modules'; -export const NUM_DIFFERENT_COLORS = 8; - -function generateInitialColors(): ColorIndex[] { - return range(NUM_DIFFERENT_COLORS); +function generateInitialColors(numOfColors: number): ColorIndex[] { + console.log(numOfColors); + return range(numOfColors); } // Returns a new index that is not present in the current color index. -// If there are more than NUM_DIFFERENT_COLORS modules already present, +// If there are more than numOfColors modules already present, // will try to balance the color distribution if randomize === true. -export function getNewColor(currentColors: ColorIndex[], randomize = true): ColorIndex { - let availableColors = generateInitialColors(); +export function getNewColor( + currentColors: ColorIndex[], + numOfColors: number, + randomize = true, +): ColorIndex { + let availableColors = generateInitialColors(numOfColors); currentColors.forEach((index: ColorIndex) => { availableColors = without(availableColors, index); if (availableColors.length === 0) { - availableColors = generateInitialColors(); + availableColors = generateInitialColors(numOfColors); } }); @@ -34,13 +37,14 @@ export function getNewColor(currentColors: ColorIndex[], randomize = true): Colo export function colorLessonsByKey( lessons: T[], key: keyof T, + numOfColors: number, ): (T & { colorIndex: ColorIndex })[] { const colorMap = new Map(); return lessons.map((lesson) => { let colorIndex = colorMap.get(lesson[key]); if (!colorMap.has(lesson[key])) { - colorIndex = getNewColor(Array.from(colorMap.values()), false); + colorIndex = getNewColor(Array.from(colorMap.values()), numOfColors, false); colorMap.set(lesson[key], colorIndex); } @@ -54,6 +58,7 @@ export function colorLessonsByKey( export function fillColorMapping( timetable: SemTimetableConfig, original: ColorMapping, + numOfColors: number, ): ColorMapping { const colorMap: ColorMapping = {}; const colorsUsed: ColorIndex[] = []; @@ -71,7 +76,7 @@ export function fillColorMapping( // Assign the modules without colors withoutColors.forEach((moduleCode) => { - const color = getNewColor(colorsUsed, false); + const color = getNewColor(colorsUsed, numOfColors, false); colorMap[moduleCode] = color; colorsUsed.push(color); }); diff --git a/website/src/views/components/ColorPicker.tsx b/website/src/views/components/ColorPicker.tsx index 12e5f5efc5..2af728830e 100644 --- a/website/src/views/components/ColorPicker.tsx +++ b/website/src/views/components/ColorPicker.tsx @@ -3,9 +3,10 @@ import classnames from 'classnames'; import Downshift, { ChildrenFunction } from 'downshift'; import _ from 'lodash'; +import { useSelector } from 'react-redux'; +import { State } from 'types/state'; import { ColorIndex } from 'types/timetables'; -import { NUM_DIFFERENT_COLORS } from 'utils/colors'; import styles from './ColorPicker.scss'; type Props = { @@ -21,6 +22,8 @@ type Props = { * For use in places like changing module colors */ const ColorPicker = memo((props) => { + const theme = useSelector((state: State) => state.theme); + const renderColorPicker: ChildrenFunction = ({ getToggleButtonProps, getItemProps, @@ -44,7 +47,7 @@ const ColorPicker = memo((props) => { className={classnames(styles.palette, { [styles.isClosed]: !isOpen })} {...getMenuProps()} > - {_.range(NUM_DIFFERENT_COLORS).map((index: ColorIndex) => ( + {_.range(theme.numOfColors).map((index: ColorIndex) => ( From b0392c7dc45e57c776ecd329fcfc6437b49ab60e Mon Sep 17 00:00:00 2001 From: joeng03 Date: Fri, 4 Aug 2023 10:53:13 +0800 Subject: [PATCH 2/8] fix: venue timetable renders white color lessons --- website/src/actions/theme.ts | 1 + website/src/actions/timetables.ts | 2 +- website/src/entry/export/TimetableOnly.tsx | 2 +- website/src/views/settings/SettingsContainer.tsx | 4 ++-- website/src/views/settings/ThemeOption.tsx | 4 ++-- website/src/views/timetable/TimetableContainer.tsx | 3 ++- website/src/views/venues/VenueDetails.tsx | 6 +++++- 7 files changed, 14 insertions(+), 8 deletions(-) diff --git a/website/src/actions/theme.ts b/website/src/actions/theme.ts index 50e97cc80a..c4d17b1d24 100644 --- a/website/src/actions/theme.ts +++ b/website/src/actions/theme.ts @@ -1,4 +1,5 @@ import { Theme } from 'types/settings'; + export const SELECT_THEME = 'SELECT_THEME' as const; export function selectTheme(theme: Theme) { return { diff --git a/website/src/actions/timetables.ts b/website/src/actions/timetables.ts index 9e862bfc49..e6ab9fda01 100644 --- a/website/src/actions/timetables.ts +++ b/website/src/actions/timetables.ts @@ -70,7 +70,7 @@ export function addModule(semester: Semester, moduleCode: ModuleCode) { const lessons = getModuleTimetable(module, semester); const moduleLessonConfig = randomModuleLessonConfig(lessons); - const numOfColors = getState().theme.numOfColors; + const { numOfColors } = getState().theme; dispatch(Internal.addModule(semester, moduleCode, moduleLessonConfig, numOfColors)); }); diff --git a/website/src/entry/export/TimetableOnly.tsx b/website/src/entry/export/TimetableOnly.tsx index 89faa7574a..b26ab5d095 100644 --- a/website/src/entry/export/TimetableOnly.tsx +++ b/website/src/entry/export/TimetableOnly.tsx @@ -29,7 +29,7 @@ export default class TimetableOnly extends Component { render() { const { store } = this.props; - const theme = store.getState().theme; + const { theme } = store.getState(); const { semester, timetable, colors } = this.state; const timetableColors = fillColorMapping(timetable, colors, theme.numOfColors); diff --git a/website/src/views/settings/SettingsContainer.tsx b/website/src/views/settings/SettingsContainer.tsx index 6819631384..e634ba3b23 100644 --- a/website/src/views/settings/SettingsContainer.tsx +++ b/website/src/views/settings/SettingsContainer.tsx @@ -3,10 +3,10 @@ import { connect } from 'react-redux'; import classnames from 'classnames'; import { isEqual } from 'lodash'; -import { Mode } from 'types/settings'; +import { Mode, Theme } from 'types/settings'; import { Tracker } from 'types/vendor/piwik'; import { ModRegNotificationSettings } from 'types/reducers'; -import { Theme } from 'types/settings'; + import { State as StoreState } from 'types/state'; import { RegPeriod, SCHEDULE_TYPES, ScheduleType } from 'config'; diff --git a/website/src/views/settings/ThemeOption.tsx b/website/src/views/settings/ThemeOption.tsx index cc63a6b94a..e7c8dcd80a 100644 --- a/website/src/views/settings/ThemeOption.tsx +++ b/website/src/views/settings/ThemeOption.tsx @@ -33,9 +33,9 @@ const ThemeOption: React.FC = (props) => { key={index} className={classnames(styles.colorItem, `color-${index}`)} style={{ - width: 100 / theme.numOfColors + '%', + width: `${100 / theme.numOfColors}%`, }} - > + /> ))} diff --git a/website/src/views/timetable/TimetableContainer.tsx b/website/src/views/timetable/TimetableContainer.tsx index 1a358830cc..d66f040d81 100644 --- a/website/src/views/timetable/TimetableContainer.tsx +++ b/website/src/views/timetable/TimetableContainer.tsx @@ -145,6 +145,7 @@ export const TimetableContainerComponent: FC = () => { const getModule = useSelector(getModuleCondensed); const modules = useSelector(({ moduleBank }: State) => moduleBank.modules); const activeSemester = useSelector(({ app }: State) => app.activeSemester); + const theme = useSelector((state: State) => state.theme); const location = useLocation(); const [importedTimetable, setImportedTimetable] = useState(() => @@ -173,7 +174,7 @@ export const TimetableContainerComponent: FC = () => { const displayedTimetable = importedTimetable || timetable; const filledColors = useMemo( - () => fillColorMapping(displayedTimetable, colors), + () => fillColorMapping(displayedTimetable, colors, theme.numOfColors), [colors, displayedTimetable], ); const readOnly = displayedTimetable === importedTimetable; diff --git a/website/src/views/venues/VenueDetails.tsx b/website/src/views/venues/VenueDetails.tsx index 3d7a93d01c..c50aa1159f 100644 --- a/website/src/views/venues/VenueDetails.tsx +++ b/website/src/views/venues/VenueDetails.tsx @@ -1,10 +1,12 @@ import { FC, memo, useCallback, useMemo } from 'react'; import { Link, useHistory } from 'react-router-dom'; +import { useSelector } from 'react-redux'; import { ChevronLeft, ChevronRight } from 'react-feather'; import classnames from 'classnames'; import { flatMap } from 'lodash'; import type { DayAvailability, TimePeriod, Venue } from 'types/venues'; +import { State } from 'types/state'; import type { Lesson } from 'types/timetables'; import { colorLessonsByKey } from 'utils/colors'; @@ -33,6 +35,8 @@ const VenueDetailsComponent: FC = ({ availability, highlightPeriod, }) => { + const theme = useSelector((state: State) => state.theme); + const arrangedLessons = useMemo(() => { const lessons: Lesson[] = flatMap(availability, (day) => day.classes).map((venueLesson) => ({ ...venueLesson, @@ -40,7 +44,7 @@ const VenueDetailsComponent: FC = ({ isModifiable: true, venue: '', })); - const coloredLessons = colorLessonsByKey(lessons, 'moduleCode'); + const coloredLessons = colorLessonsByKey(lessons, 'moduleCode', theme.numOfColors); return arrangeLessonsForWeek(coloredLessons); }, [availability]); From 8563458617505d52d9145c2bb450214c512cba22 Mon Sep 17 00:00:00 2001 From: joeng03 Date: Fri, 4 Aug 2023 13:51:14 +0800 Subject: [PATCH 3/8] fix: linting errors --- website/src/utils/colors.ts | 1 - website/src/views/timetable/TimetableContainer.tsx | 6 +++--- website/src/views/venues/VenueDetails.tsx | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/website/src/utils/colors.ts b/website/src/utils/colors.ts index ae8b645e8f..83df637ea2 100644 --- a/website/src/utils/colors.ts +++ b/website/src/utils/colors.ts @@ -5,7 +5,6 @@ import { ColorMapping } from 'types/reducers'; import { ModuleCode } from 'types/modules'; function generateInitialColors(numOfColors: number): ColorIndex[] { - console.log(numOfColors); return range(numOfColors); } diff --git a/website/src/views/timetable/TimetableContainer.tsx b/website/src/views/timetable/TimetableContainer.tsx index d66f040d81..2b170152e8 100644 --- a/website/src/views/timetable/TimetableContainer.tsx +++ b/website/src/views/timetable/TimetableContainer.tsx @@ -145,7 +145,7 @@ export const TimetableContainerComponent: FC = () => { const getModule = useSelector(getModuleCondensed); const modules = useSelector(({ moduleBank }: State) => moduleBank.modules); const activeSemester = useSelector(({ app }: State) => app.activeSemester); - const theme = useSelector((state: State) => state.theme); + const numOfColors = useSelector(({ theme }: State) => theme.numOfColors); const location = useLocation(); const [importedTimetable, setImportedTimetable] = useState(() => @@ -174,8 +174,8 @@ export const TimetableContainerComponent: FC = () => { const displayedTimetable = importedTimetable || timetable; const filledColors = useMemo( - () => fillColorMapping(displayedTimetable, colors, theme.numOfColors), - [colors, displayedTimetable], + () => fillColorMapping(displayedTimetable, colors, numOfColors), + [colors, numOfColors, displayedTimetable], ); const readOnly = displayedTimetable === importedTimetable; diff --git a/website/src/views/venues/VenueDetails.tsx b/website/src/views/venues/VenueDetails.tsx index c50aa1159f..4132567f5c 100644 --- a/website/src/views/venues/VenueDetails.tsx +++ b/website/src/views/venues/VenueDetails.tsx @@ -35,7 +35,7 @@ const VenueDetailsComponent: FC = ({ availability, highlightPeriod, }) => { - const theme = useSelector((state: State) => state.theme); + const numOfColors = useSelector(({ theme }: State) => theme.numOfColors); const arrangedLessons = useMemo(() => { const lessons: Lesson[] = flatMap(availability, (day) => day.classes).map((venueLesson) => ({ @@ -44,9 +44,9 @@ const VenueDetailsComponent: FC = ({ isModifiable: true, venue: '', })); - const coloredLessons = colorLessonsByKey(lessons, 'moduleCode', theme.numOfColors); + const coloredLessons = colorLessonsByKey(lessons, 'moduleCode', numOfColors); return arrangeLessonsForWeek(coloredLessons); - }, [availability]); + }, [availability, numOfColors]); const history = useHistory(); const navigateToLesson = useCallback( From bf7e00622553729cc04f2c1cabd1f122a4348bb6 Mon Sep 17 00:00:00 2001 From: joeng03 Date: Sun, 6 Aug 2023 23:46:32 +0800 Subject: [PATCH 4/8] refactor: refactor test cases to reflect current code modifications --- .../src/actions/__snapshots__/theme.test.ts.snap | 6 +++++- website/src/actions/theme.test.ts | 7 ++++++- website/src/reducers/index.test.ts | 2 ++ website/src/reducers/theme.test.ts | 9 +++++++-- website/src/views/components/ColorPicker.test.tsx | 15 +++++++++++++++ 5 files changed, 35 insertions(+), 4 deletions(-) diff --git a/website/src/actions/__snapshots__/theme.test.ts.snap b/website/src/actions/__snapshots__/theme.test.ts.snap index 670e33f8cd..8f0967c0ea 100644 --- a/website/src/actions/__snapshots__/theme.test.ts.snap +++ b/website/src/actions/__snapshots__/theme.test.ts.snap @@ -16,7 +16,11 @@ exports[`theme should dispatch a cycle of theme 2`] = ` exports[`theme should dispatch a select of theme 1`] = ` { - "payload": "test", + "payload": { + "id": "test", + "name": "Test", + "numOfColors": 8, + }, "type": "SELECT_THEME", } `; diff --git a/website/src/actions/theme.test.ts b/website/src/actions/theme.test.ts index 37a33e5f3e..1a8bfb7ecc 100644 --- a/website/src/actions/theme.test.ts +++ b/website/src/actions/theme.test.ts @@ -1,8 +1,13 @@ import * as actions from 'actions/theme'; +import { Theme } from 'types/settings'; describe('theme', () => { test('should dispatch a select of theme', () => { - const theme = 'test'; + const theme: Theme = { + id: 'test', + name: 'Test', + numOfColors: 8, + }; expect(actions.selectTheme(theme)).toMatchSnapshot(); }); diff --git a/website/src/reducers/index.test.ts b/website/src/reducers/index.test.ts index ca12eece52..33e476caa4 100644 --- a/website/src/reducers/index.test.ts +++ b/website/src/reducers/index.test.ts @@ -30,6 +30,7 @@ const exportData: ExportData = { hidden: ['PC1222'], theme: { id: 'google', + numOfColors: 8, timetableOrientation: VERTICAL, showTitle: true, }, @@ -83,6 +84,7 @@ test('reducers should set export data state', () => { expect(state.theme).toEqual({ id: 'google', + numOfColors: 8, timetableOrientation: VERTICAL, showTitle: true, }); diff --git a/website/src/reducers/theme.test.ts b/website/src/reducers/theme.test.ts index 9fc7049919..390abb42f1 100644 --- a/website/src/reducers/theme.test.ts +++ b/website/src/reducers/theme.test.ts @@ -2,10 +2,15 @@ import { ThemeState, VERTICAL } from 'types/reducers'; import * as actions from 'actions/theme'; import reducer, { defaultThemeState, themeIds } from 'reducers/theme'; +import { Theme } from 'types/settings'; const themeInitialState: ThemeState = defaultThemeState; -const googleTheme = 'google'; -const themeWithEighties: ThemeState = { ...themeInitialState, id: googleTheme }; +const googleTheme: Theme = { + id: 'google', + name: 'Google', + numOfColors: 8, +}; +const themeWithEighties: ThemeState = { ...themeInitialState, id: googleTheme.id }; const themeWithFirstTheme: ThemeState = { ...themeInitialState, id: themeIds[0] }; const themeWithLastTheme: ThemeState = { ...themeInitialState, id: themeIds[themeIds.length - 1] }; const themeWithVerticalOrientation: ThemeState = { diff --git a/website/src/views/components/ColorPicker.test.tsx b/website/src/views/components/ColorPicker.test.tsx index d4dfe6724e..93f62d8e49 100644 --- a/website/src/views/components/ColorPicker.test.tsx +++ b/website/src/views/components/ColorPicker.test.tsx @@ -1,16 +1,31 @@ +import configureStore from 'bootstrapping/configure-store'; import { mount, ReactWrapper } from 'enzyme'; import ColorPicker from 'views/components/ColorPicker'; import { ColorIndex } from 'types/timetables'; import { expectColor } from 'test-utils/theme'; +import { initAction } from 'test-utils/redux'; +import { Provider } from 'react-redux'; +import { PropsWithChildren } from 'react'; +import reducers from 'reducers'; import styles from './ColorPicker.scss'; function makeColorPicker(color: ColorIndex = 0) { const onChooseColor = jest.fn(); + const initialState = reducers(undefined, initAction()); + const { store } = configureStore(initialState); + + const ProviderWrapper = ({ children }: PropsWithChildren>) => ( + {children} + ); + return { onChooseColor, wrapper: mount( , + { + wrappingComponent: ProviderWrapper, + }, ), }; } From aeafe7c41538eaf3e422caa5c039ef254b173f2a Mon Sep 17 00:00:00 2001 From: joeng03 Date: Mon, 7 Aug 2023 09:24:31 +0800 Subject: [PATCH 5/8] feat: add action reassignAllModulesColor to reassign the color of the modules after selectTheme --- website/src/actions/timetables.ts | 10 +++++++++ website/src/reducers/theme.ts | 1 + website/src/reducers/timetables.ts | 22 +++++++++++++++++-- .../src/views/settings/SettingsContainer.tsx | 8 ++++++- website/src/views/settings/ThemeOption.tsx | 4 +++- 5 files changed, 41 insertions(+), 4 deletions(-) diff --git a/website/src/actions/timetables.ts b/website/src/actions/timetables.ts index e6ab9fda01..ad2745b8f2 100644 --- a/website/src/actions/timetables.ts +++ b/website/src/actions/timetables.ts @@ -214,6 +214,16 @@ export function selectModuleColor( }; } +export const REASSIGN_ALL_MODULES_COLOR = 'REASSIGN_ALL_MODULES_COLOR' as const; +export function reassignAllModulesColor(numOfColors: number) { + return { + type: REASSIGN_ALL_MODULES_COLOR, + payload: { + numOfColors, + }, + }; +} + export const HIDE_LESSON_IN_TIMETABLE = 'HIDE_LESSON_IN_TIMETABLE' as const; export function hideLessonInTimetable(semester: Semester, moduleCode: ModuleCode) { return { diff --git a/website/src/reducers/theme.ts b/website/src/reducers/theme.ts index 400c09ed0d..93f741bdce 100644 --- a/website/src/reducers/theme.ts +++ b/website/src/reducers/theme.ts @@ -35,6 +35,7 @@ function theme(state: ThemeState = defaultThemeState, action: Actions): ThemeSta switch (action.type) { case SELECT_THEME: + // Reassign all modules' color when changing theme return setTheme(action.payload); case CYCLE_THEME: { diff --git a/website/src/reducers/timetables.ts b/website/src/reducers/timetables.ts index fbdf606558..ce173012fc 100644 --- a/website/src/reducers/timetables.ts +++ b/website/src/reducers/timetables.ts @@ -5,7 +5,7 @@ import { createMigrate } from 'redux-persist'; import { PersistConfig } from 'storage/persistReducer'; import { ModuleCode } from 'types/modules'; import { ModuleLessonConfig, SemTimetableConfig } from 'types/timetables'; -import { ColorMapping, TimetablesState } from 'types/reducers'; +import { ColorMapping, SemesterColorMap, TimetablesState } from 'types/reducers'; import config from 'config'; import { @@ -14,6 +14,7 @@ import { HIDE_LESSON_IN_TIMETABLE, REMOVE_MODULE, SELECT_MODULE_COLOR, + REASSIGN_ALL_MODULES_COLOR, SET_LESSON_CONFIG, SET_TIMETABLE, SHOW_LESSON_IN_TIMETABLE, @@ -148,6 +149,19 @@ function semColors(state: ColorMapping = defaultSemColorMap, action: Actions): C } } +function recolor(state: SemesterColorMap, numOfColors: number): SemesterColorMap { + const newSemesterColorMap: SemesterColorMap = {}; + Object.keys(state).forEach((semester) => { + const colorMapping = state[semester]; + const newColorMapping: ColorMapping = {}; + Object.keys(colorMapping).forEach((moduleCode) => { + newColorMapping[moduleCode] = colorMapping[moduleCode] % numOfColors; + }); + newSemesterColorMap[semester] = newColorMapping; + }); + return newSemesterColorMap; +} + // Map of semester to list of hidden modules const defaultHiddenState: ModuleCode[] = []; function semHiddenModules(state = defaultHiddenState, action: Actions) { @@ -182,7 +196,6 @@ function timetables( if (!action.payload) { return state; } - switch (action.type) { case SET_TIMETABLE: { const { semester, timetable, colors } = action.payload; @@ -219,6 +232,11 @@ function timetables( hidden: { [semester]: hidden }, }; } + case REASSIGN_ALL_MODULES_COLOR: + return { + ...state, + colors: recolor(state.colors, action.payload.numOfColors), + }; default: return state; diff --git a/website/src/views/settings/SettingsContainer.tsx b/website/src/views/settings/SettingsContainer.tsx index e634ba3b23..5746cdd500 100644 --- a/website/src/views/settings/SettingsContainer.tsx +++ b/website/src/views/settings/SettingsContainer.tsx @@ -12,6 +12,7 @@ import { RegPeriod, SCHEDULE_TYPES, ScheduleType } from 'config'; import availableThemes from 'data/themes.json'; import { selectTheme } from 'actions/theme'; +import { reassignAllModulesColor } from 'actions/timetables'; import { dismissModregNotification, enableModRegNotification, @@ -50,6 +51,7 @@ type Props = { selectTheme: (theme: Theme) => void; selectMode: (mode: Mode) => void; + reassignAllModulesColor: (numOfColors: number) => void; toggleBetaTesting: () => void; setLoadDisqusManually: (status: boolean) => void; @@ -140,7 +142,10 @@ const SettingsContainer: React.FC = ({ className={styles.themeOption} theme={theme} isSelected={currentThemeId === theme.id} - onSelectTheme={props.selectTheme} + onSelectTheme={(theme) => { + props.selectTheme(theme); + props.reassignAllModulesColor(theme.numOfColors); + }} /> ))}
@@ -306,6 +311,7 @@ const connectedSettings = connect(mapStateToProps, { selectTheme, selectFaculty, selectMode, + reassignAllModulesColor, toggleBetaTesting, setLoadDisqusManually, toggleModRegNotificationGlobally, diff --git a/website/src/views/settings/ThemeOption.tsx b/website/src/views/settings/ThemeOption.tsx index e7c8dcd80a..9d6e4c4be8 100644 --- a/website/src/views/settings/ThemeOption.tsx +++ b/website/src/views/settings/ThemeOption.tsx @@ -22,7 +22,9 @@ const ThemeOption: React.FC = (props) => { className={classnames(className, styles.option, `theme-${theme.id}`, { [styles.isSelected]: isSelected, })} - onClick={() => onSelectTheme(theme)} + onClick={() => { + onSelectTheme(theme); + }} >
{theme.name} From ef2620101168390ecd4d74e36782b1b3eff5e17a Mon Sep 17 00:00:00 2001 From: joeng03 Date: Mon, 7 Aug 2023 10:03:34 +0800 Subject: [PATCH 6/8] fix(SettingsContainer): eslint.no-shadow --- website/src/views/settings/SettingsContainer.tsx | 2 +- website/src/views/settings/ThemeOption.tsx | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/website/src/views/settings/SettingsContainer.tsx b/website/src/views/settings/SettingsContainer.tsx index 5746cdd500..f7e653018b 100644 --- a/website/src/views/settings/SettingsContainer.tsx +++ b/website/src/views/settings/SettingsContainer.tsx @@ -142,7 +142,7 @@ const SettingsContainer: React.FC = ({ className={styles.themeOption} theme={theme} isSelected={currentThemeId === theme.id} - onSelectTheme={(theme) => { + onSelectTheme={() => { props.selectTheme(theme); props.reassignAllModulesColor(theme.numOfColors); }} diff --git a/website/src/views/settings/ThemeOption.tsx b/website/src/views/settings/ThemeOption.tsx index 9d6e4c4be8..e97eea2e95 100644 --- a/website/src/views/settings/ThemeOption.tsx +++ b/website/src/views/settings/ThemeOption.tsx @@ -9,7 +9,7 @@ import styles from './ThemeOption.scss'; type Props = { theme: Theme; isSelected: boolean; - onSelectTheme: (theme: Theme) => void; + onSelectTheme: () => void; className?: string; }; @@ -22,9 +22,7 @@ const ThemeOption: React.FC = (props) => { className={classnames(className, styles.option, `theme-${theme.id}`, { [styles.isSelected]: isSelected, })} - onClick={() => { - onSelectTheme(theme); - }} + onClick={onSelectTheme} >
{theme.name} From b97aa2683cf507b6fb1e919a75ca6dbe3e245755 Mon Sep 17 00:00:00 2001 From: joeng03 Date: Thu, 10 Aug 2023 19:28:27 +0800 Subject: [PATCH 7/8] fix: lint error --- website/src/views/settings/ThemeOption.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/website/src/views/settings/ThemeOption.scss b/website/src/views/settings/ThemeOption.scss index 0a424ccfce..bcfdfc3675 100644 --- a/website/src/views/settings/ThemeOption.scss +++ b/website/src/views/settings/ThemeOption.scss @@ -1,5 +1,3 @@ -@use "sass:math"; - @import '~styles/utils/modules-entry'; $theme-selected-bg-color: var(--gray-lighter); From a4304f45c60b8126695c03dc1a296a0eaf95bd5e Mon Sep 17 00:00:00 2001 From: joeng03 Date: Sun, 13 Aug 2023 13:11:23 +0800 Subject: [PATCH 8/8] refactor: address changes requested --- website/src/reducers/timetables.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/website/src/reducers/timetables.ts b/website/src/reducers/timetables.ts index 241ff0b7ef..6db5744e50 100644 --- a/website/src/reducers/timetables.ts +++ b/website/src/reducers/timetables.ts @@ -149,13 +149,12 @@ function semColors(state: ColorMapping = defaultSemColorMap, action: Actions): C } } -function recolor(state: SemesterColorMap, numOfColors: number): SemesterColorMap { +function recolor(curSemesterColorMap: SemesterColorMap, numOfColors: number): SemesterColorMap { const newSemesterColorMap: SemesterColorMap = {}; - Object.keys(state).forEach((semester) => { - const colorMapping = state[semester]; + Object.entries(curSemesterColorMap).forEach(([semester, colorMapping]) => { const newColorMapping: ColorMapping = {}; - Object.keys(colorMapping).forEach((moduleCode) => { - newColorMapping[moduleCode] = colorMapping[moduleCode] % numOfColors; + Object.entries(colorMapping).forEach(([moduleCode, colorIndex]) => { + newColorMapping[moduleCode] = colorIndex % numOfColors; }); newSemesterColorMap[semester] = newColorMapping; }); @@ -196,6 +195,7 @@ function timetables( if (!action.payload) { return state; } + switch (action.type) { case SET_TIMETABLE: { const { semester, timetable, colors } = action.payload;