Feat seasonal calender #185
Conversation
|
TODO: |
| @@ -7,6 +7,16 @@ export const ROUTES_COMMON: Routes = [ | |||
| loadChildren: () => import('./pages/home/home.module').then((m) => m.HomeModule), | |||
| title: 'Seasonal Calendar', | |||
| }, | |||
| { | |||
| path: 'create-calendar', | |||
There was a problem hiding this comment.
nit(non-blocking)
The url will already have /seasonal-calendar/ so probably can just call the child routes /create and /[calendarId] to view
There was a problem hiding this comment.
Alright, thanks for this @chrismclarke
| title: 'Create Calendar', | ||
| }, | ||
| { | ||
| path: 'calendar-table', |
There was a problem hiding this comment.
nit(non-blocking)
The urls might look a bit cleaner if instead of this path you used a route paramater, e.g. path: ':id' which will make the the id available on page load. E.g. picsa.app/seasonal-calendar/abcd123 will be able to load info for seasonal calendar with local id abcd123
https://angular.io/guide/router-reference#configuration
There was a problem hiding this comment.
Yes, Ill try to handle that issue aswell
|
|
||
| constructor( | ||
| public dialogRef: MatDialogRef<ActivitiesEditorDialogComponent>, | ||
| @Inject(MAT_DIALOG_DATA) public data: any |
There was a problem hiding this comment.
nice to see you were able to figure out passing data in/out of dialogs
| } | ||
|
|
||
| onSubmition(){ | ||
| if(this.calendarMonths.length > 0 && this.userCrops.length > 0 && this.calenderTitle){ |
There was a problem hiding this comment.
nit(non-blocking)
Custom validation doesn't always provide the nicest user experience as it doesn't track exactly which input is lacking and identify to the user on screen (e.g. showing a red box around missing input)
Angular has a lot of in-built tools for creating forms and validators. It might be worth thinking in the future to refactor to use a reactive form
https://angular.io/guide/reactive-forms
There was a problem hiding this comment.
Going to handle this too, thanks for the guidence
| // https://rxdb.info/data-migration.html | ||
| migrationStrategies: { | ||
| ...COLLECTION_V0.migrationStrategies, | ||
| 1: (data: CalendarDataEntry_v0): CalendarDataEntry_v1 => { |
There was a problem hiding this comment.
really nice to see not only were you able to setup a database schema, but also provide migration strategy to update local data as you developed the schema. It's a really effective strategy when working with any type of database
There was a problem hiding this comment.
Lots of really nice progress here. I've gone ahead created a separate pr to add support for shared form input components (e.g. crop probability select), #195, and added commit 5b45724 to integrate into the seasonal calendar tool which simplifies some of the input logic nicely.
I've also added one additional commit 645b779 with a small amount of code tidying
See a few comments in-line, but overall happy to merge here and continue to build in follow-up PRs, nice work!
|
Thanks for the detailed review, it is really helpful. |
chrismclarke
added
the
Tool: Seasonal Calendar
Description
This PR is to prototype the seasonal calendar's initial functionality.
Discussion
Dev meeting 5th, Oct 2023
Preview
Screenshots / Videos