-
Notifications
You must be signed in to change notification settings - Fork 191
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
Normalized trackers refactor (+AniList) #631
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mostly functional. Many large, sweeping changes to the data model used, so it may be worth looking into migrating the existing data structures. Otherwise it's a bad time for people already using MAL to track. Much of the data model is documented with JSDoc, mostly for my sanity. I've never used JSDoc before so idk how correct these defs are 🤷 MAL has been mostly migrated over to the new format. There's a few straggling bits that seem to reference the old code that need to be cleared out. Things to do: * Status selector needs to respect AniList values/options when AL is in use * Rating selector should respect the user's chosen rating format * AniList option in the Discover section
Fixes issues where things are initialized in the wrong order
MAL is effectively unchanged AL supports all available formats. The UI leaves a bit to be desired, but it's functional.
Additional cleanup pass
rajarsheechatterjee
approved these changes
May 14, 2023
Tested MAL, LGTM. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #42 (Example videos can be found in the replies)
Refactors the tracker system to be normalized for easy support of many different trackers. User data is migrated to the new data format when they launch the app for the first time since this update.
This was designed mostly with MyAnimeList and AniList in mind, but should be applicable to other trackers as well.
I tested everything I can think of, but I'm not a MAL user (and MAL is dead at the time of opening this PR 🙄) so it could definitely use a once over by another set of eyes. The data migration part could definitely use a solid test.
The
MalTopNovels
andAniListTopNovels
components can probably be normalized as well, but I'm no React guy so I'm not sure how to "correctly" compose those.I did not go through and add typing to most of the tracker store, but the new storage model looks like this: