Added heart icons in settings and synced heart icon state with star rating changes #785
Conversation
the settings save and connects to html variables
also unhearting sets the rating to 0.5 as expected closest MVP, will ask Boris for help on making change rating in meta.component trigger update
merging the favorite
Merging all new changes related to favorites feature on main branch
| @@ -82,7 +82,7 @@ | |||
| "@typescript-eslint/eslint-plugin": "5.45.0", | |||
| "@typescript-eslint/parser": "5.45.0", | |||
| "electron": "22.0.0", | |||
| "electron-builder": "23.6.0", | |||
| "electron-builder": "^22.10.3", | |||
There was a problem hiding this comment.
Did it turn out 23.6.0 version was giving errors, or is this accidental?
If easy enough, would you be willing to reset the package.json and package-lock.json files to their state in the main branch?
|
|
||
| //Check if the video is "favorited" or not at the start of the hub | ||
|
|
||
| function isFavorite(stars): boolean { |
There was a problem hiding this comment.
Function code works as desired, but consider this shorter one-line version:
return stars === 5.5;ps - consider using the JSDoc formatting for the function description / comment: JSDoc
| height: 50%; | ||
| position: absolute; | ||
| width: 50%; | ||
| height: 50%; |
There was a problem hiding this comment.
Not too important, but could we go back to the 2-space indentation we had before? 😊
| @@ -71,4 +77,24 @@ export class FullViewComponent implements OnInit { | |||
| this.rowOffsets.push(i * Math.floor(this._metaWidth / imgWidth)); | |||
| } | |||
| } | |||
|
|
|||
| toggleHeart(): void { | |||
There was a problem hiding this comment.
I see the toggleHeart method is repeated across several components 🤔
I think we could put this code into a service and just call the service from each of the components where the functionality is needed 🤔
You could create a dedicated new service, but the imageElementService seems like a great place for this code to live in 😁
This way we can keep it DRY 😉
|
Sorry for the delay -- I'll try to find some time this week (before 2022 is over) to merge in your code ❤️ |
|
Thank you again for the code addition 🙇 |
|
🙇 thank you for the code contribution -- I've merged it into Your code should appear in the 5th anniversary release happening later today 🙌 Cheers! |
Does not fully fulfill requirements stated in #763
Hello Boris!
Thank you for your support so far.
This pull request contains the following:
If you have insights on why it is not working for other views, we would like to know!
Thank you!