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.

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

Dont store scheduling source as date in redux #268

Merged

Conversation

Arnei
Copy link
Member

@Arnei Arnei commented Jan 11, 2024

While storing dates as date objects in redux does not necessarily break anything, it can interfere with some features (persisting, debugging) and lead to bad practices (for an encompassing discussion see reduxjs/redux-toolkit#456).

This changes the scheduling info fetching method to store dates as strings.

While storing dates as date objects in redux does not
necessarily break anything, it can interfere with some
features (persisting, debugging) and lead to bad practices
(for an encompassing discussion see reduxjs/redux-toolkit#456).

This changes the scheduling info fetching method
to store dates as strings
@Arnei Arnei added the type:code-enhancement Internal improvements to the codebase label Jan 11, 2024
@@ -871,12 +871,12 @@ export const fetchSchedulingInfo = (eventId) => async (dispatch, getState) => {
const source = {
...schedulingResponse,
start: {
date: startDate,
date: schedulingResponse.start,
Copy link
Member

Choose a reason for hiding this comment

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

So, for me these render as (DD/MM/YY HH:MM) when the modal opens, but then clicking on them to change them switches to the long date (Feburary 22nd 11pm). This is quite confusing IMO - I would not expect the format to change when clicking in.

Copy link
Contributor

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

Copy link
Contributor

github-actions bot commented Mar 4, 2024

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@gregorydlogan gregorydlogan self-assigned this May 24, 2024
@gregorydlogan gregorydlogan merged commit a3a7873 into opencast:main May 24, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:code-enhancement Internal improvements to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants