-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat(seasonal-calendar): v2 refactor #222
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
…actor/seasonal-calendar-data
…actor/seasonal-calendar-data
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
App: Dashboard
Updates related to Dashboard app
Feedback Discussion
Test - Preview
Tool: Seasonal Calendar
Updates related to Seasonal Calendar tool
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.
Description
This PR turned into a full rewrite of the tool to trial improved methods of data handling.
The overall UI and functionality of the tool has been improved to contain additional visual methods for selection and editing, and should now hopefully be ready for production use. See demo video below for more info
Review Notes
@gkebirungi
I'm hoping that this should now be in a position to share with the UoR team, however before we do I'd be keen if you could take a once-over to see if things work well for you.
I'd encourage trying to test the full functionality if possible (e.g. create new calendar, input data, make edits, delete etc.), and keep an eye out for anything that you think might be confusing for users. You should be able to access latest code via the link below:
https://picsa-extension-toolkit--pr222-refactor-seasonal-ca-wz3ybnkr.web.app/
Developer Notes
@khalifan-kfan
Making sense of the PR file changes will be pretty much impossible due to the scope and scale of changes implemented. I attempted splitting into smaller prs but this turned out to be more effort than it was worth.
As such I'd be interested if you could instead just take a quick look at the code with a fresh perspective and see if things make sense to you (are developer friendly). I would recommend rough order of files:
apps\picsa-tools\seasonal-calendar-tool\src\app\schema\schema_v0.ts
This contains the new data schema. It should be simpler than the previous (less nested and less duplication). It also adds support for future non-crop enterprises (e.g. livestock).
I had originally defined as data migration, however realised the db name needed updating anyways due to a minor typo (
calender
->calendar
), so thought better to just start from scratch. You might need to clear your local cache if this causes any issues.apps\picsa-tools\seasonal-calendar-tool\src\app\services\calendar-form.service.ts
This is a new service that converts the db schema into a form, for use with angular forms editing. Once the schema is within a form it becomes a lot more easy to handle things like data validation and knock-on effects - no need for custom code within components or db service, everything can be handled by form logic instead.
apps\picsa-tools\seasonal-calendar-tool\src\app\services\calendar.data.service.ts
This is the original service used to interact with the db, although now much simpler as it doesn't have to handle any complicated data manipulation
apps\picsa-tools\seasonal-calendar-tool\src\app\components\calendar-editor
This is the main component used to specify initial calendar parameters (e.g. name, crops and months included). You'll notice that no additional methods are required to save data, instead all inputs can be bound directly to form controls which handle all the save/update logic.
apps\picsa-tools\seasonal-calendar-tool\src\app\pages
These are the rest of the pages, they're mostly unchanged from before (still home, create, and table), but again have had most data manipulation removed and replaced instead with form service integration.
I'm hoping that the structures above come across as reasonably understandable as moving forwards I think it might be good to specify more of the frontend code in similar ways (hopefully easier to maintain in the long term). So any feedback you have would be most welcome.
Thanks also for all the work you've put into the tool, really made it a lot easier to build on and test different strategies.
Discussion
UoR Feedback
Preview
Link to app preview if relevant
Screenshots / Videos
Demo Video
PICSA.webm
Changes since recorded:
Create Calendar
button size fix