Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Refactor/more redux reducers #622

Merged
merged 35 commits into from
Feb 8, 2022
Merged

Refactor/more redux reducers #622

merged 35 commits into from
Feb 8, 2022

Conversation

JiaqiWang18
Copy link
Member

@JiaqiWang18 JiaqiWang18 commented Feb 1, 2022 •
edited
Loading

Description

Refactor more redux reducers

Progress

Current Progress: 11 completed, 8 left to be completed in another PR

Change Log

Refactored the following reducer into redux toolkit slice

  • Preferences reducer
  • Registrar reducer
  • Save Calendar Modal reducer
  • School reducer
  • Search Results reducer
  • Semester reducer
  • Terms of Service Banner reducer
  • Terms of Service Modal reducer
  • Ui reducer

@JiaqiWang18 JiaqiWang18 marked this pull request as ready for review February 5, 2022 23:09
@JiaqiWang18 JiaqiWang18 marked this pull request as draft February 5, 2022 23:10
@JiaqiWang18
Copy link
Member Author

seems like GitHub actions is down

@JJamesWWang
Copy link
Member

This is gonna be a massive PR to review if you finish it al; can you mark this one as ready and create another one?

@JiaqiWang18 JiaqiWang18 self-assigned this Feb 8, 2022
@JiaqiWang18 JiaqiWang18 added the javascript Pull requests that update Javascript code label Feb 8, 2022
@JiaqiWang18 JiaqiWang18 marked this pull request as ready for review February 8, 2022 16:58
@JJamesWWang
Copy link
Member

JJamesWWang commented Feb 8, 2022 •
edited
Loading

Could you also note down which reducers were refactored in the description?
(actually, just check if I messed any)

JJamesWWang
JJamesWWang reviewed Feb 8, 2022
View reviewed changes
static/js/redux/actions/timetable_actions.jsx
@@ -146,7 +147,7 @@ export const lockTimetable = (timetable) => (dispatch, getState) => {
const state = getState();

if (timetable.has_conflict) {
dispatch({ type: ActionTypes.TURN_CONFLICTS_ON });
dispatch(preferencesActions.turnConflictsOn());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking of getting rid of the preferences modal soon, what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the preference modal is kinda buggy, I think we should remove

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's do that after redux reducers are refactored

JJamesWWang
JJamesWWang approved these changes Feb 8, 2022
View reviewed changes
Copy link
Member

@JJamesWWang JJamesWWang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, check if the description covers everything and comment if it's right

@JiaqiWang18
Copy link
Member Author

Could you also note down which reducers were refactored in the description? (actually, just check if I messed any)

Thanks lol, I think that's all

@JiaqiWang18
Copy link
Member Author

Looks good, check if the description covers everything and comment if it's right

It's good

@JJamesWWang JJamesWWang merged commit 64d8d50 into develop Feb 8, 2022
@JJamesWWang JJamesWWang deleted the refactor/more-redux-reducers branch March 28, 2022 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JJamesWWang JJamesWWang JJamesWWang approved these changes

@yesterd3v yesterd3v Awaiting requested review from yesterd3v

Assignees

@JiaqiWang18 JiaqiWang18

Labels
javascript Pull requests that update Javascript code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JiaqiWang18 @JJamesWWang