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

Migrate Maps to RTK #1145

Open
ChrisMart21 opened this issue Mar 7, 2024 · 2 comments · May be fixed by #1314
Open

Migrate Maps to RTK #1145

ChrisMart21 opened this issue Mar 7, 2024 · 2 comments · May be fixed by #1314
Assignees
Labels
p-high-priority This issue should be the focus of design and developement effort. t-enhancement This issues tracks a potential improvement to the software
Milestone

Comments

@ChrisMart21
Copy link
Contributor

ChrisMart21 commented Mar 7, 2024

Issue Description:

A subset of #1146
While PR #1113 made progress in migrating to Redux Toolkit (RTK), the Maps component still requires updates. Immutable checks in development mode are passing successfully everywhere except within the maps. Additionally, the dev mode serializable check is being violated due to the storage of an HTMLImageElement in the state. This issue needs prompt resolution.

Steps to Reproduce:

  1. Uncomment the immutability check in store.ts by removing immutableCheck: false.
  2. Uncomment the serializable check in store.ts by removing serializableCheck: false.
  3. Inspect browser console.logs for errors.

Expected Behavior:

The application should encounter no such issues, and it should be safe to enable all dev mode checks while utilizing newer RTK patterns. It is believed that migrating this section of the codebase to RTK will resolve these issues. May also impact #1140

@ChrisMart21 ChrisMart21 converted this from a draft issue Mar 7, 2024
@ChrisMart21 ChrisMart21 added the p-high-priority This issue should be the focus of design and developement effort. label Mar 7, 2024
@ChrisMart21 ChrisMart21 moved this from Todo to In Progress in Redux Toolkit Mar 7, 2024
@ChrisMart21 ChrisMart21 self-assigned this Mar 7, 2024
@ChrisMart21 ChrisMart21 added the t-enhancement This issues tracks a potential improvement to the software label Mar 7, 2024
@ChrisMart21
Copy link
Contributor Author

Initial maps migration has begun, and can be tracked on mapsRTK
A solution to the issue of an HTMLImageELEMENT in state has been identified, and is nolonger needed for the mapChartComponent, Work on the admin pages has yet to begin.

Since the majority of maps is mainly admin page-related, other non-map admin pages will be getting some changes as well. The existing pages/modals will remain relatively unchanged with minor modifications.
Currently admin pages will create a modal for each entity. Which can cause the subscription count to the store unnecessarily high.
The idea is to use single modal whose body/contents will change as needed, eliminating the issue of excessive selector subscriptions.

Local edits will live in their own state slice, and no longer utilize React's useState.

These changes aim to improve persistence and feedback for admin users when editing data locally.
These changes also tie closely into the Unsaved Warning Component rework which was omitted in the initial push towards RTK.

@huss
Copy link
Member

huss commented Apr 3, 2024

Map image Redux state may need fixing as they are impacting dev check for map.image as HTML. Some actions that may not be used when done with the conversion to RTK.

@hazeltonbw hazeltonbw linked a pull request Aug 1, 2024 that will close this issue
5 tasks
@ChrisMart21 ChrisMart21 linked a pull request Aug 13, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p-high-priority This issue should be the focus of design and developement effort. t-enhancement This issues tracks a potential improvement to the software
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

2 participants