search

nextcloud / cookbook

🍲 A library for all your recipes
https://apps.nextcloud.com/apps/cookbook
GNU Affero General Public License v3.0
532 stars 91 forks source link

Front-end using Vue.js #172

Closed sam-19 closed 4 years ago

sam-19 commented 4 years ago

Hello,

First of all, thank you for the great app! I started working on something similar a while ago, but I actually like your approach better. I could imagine myself using this app a lot.

There is one issue I would like to bring to your attention, though. For a good while now, Nextcloud seems to have been making a big move towards using Vue.js to handle the front-end. Their support for traditional app design i still good, but for the long term (and for consistency with the rest of the infrastructure), it would be good to consider using Vue.js in this app as well. The UI part of the app is still quite simple and the undertaking would not be overwhelming at this point. Having a couple of Nextcloud app projects using Vue.js myself, I can also tell you that the component-based design can really make things easier when you want to add dynamic features to you UI (such as the suggested recalculation for different amounts of servings). Nextcloud also supplies a lot of ready-to-use Vue.js components that have helped me in my earlier projects.

I'm a bit busy at the moment, but since I can consider putting the plans for my own recipe app to rest now, I could help you with the Vue.js rework in a couple of weeks. If you want to go down that road, that is.

Best regards, Sam

PS: I have a couple of other ideas/suggestions for features for the app, such as subheaders in the ingredients section to improve readability in long ingredient lists, but I think I will return to those later. Depending on the direction you decide to take, I may be able to make pull requests for some of the features myself.

christianlupus commented 4 years ago

Hello @sam-19, I highly suggest that you join the matrix room #partkeepr:matrix.org. There some dev related discussion is much simpler to be done. I have no saying in it but personally I think this is a good suggestion.

mrzapp commented 4 years ago

@christianlupus you certainly have a say in it, you're a contributor :grin:

@sam-19 I welcome this change, if it's something that the rest of NextCloud is moving towards, let's go for it. I'll make it part of the 0.7.0 milestone.

I'll create a v0.7 branch, you can submit your pull requests there

mcorteel commented 4 years ago

I considered the idea when doing the redesign because I agree that this is clearly Nextcloud's direction. But I have zero experience with Vue to do the ground work. But when done, I guess my React experience will make it easier to get up to speed. And I definitely agree that using this type of framework increases possibilities tenfold!

TheMBeat commented 4 years ago

That's a great idea. Additionally we could use or extend this JS Recipe Scraper. The JSON output doesn't quite fit yet but could be converted.

mrzapp commented 4 years ago

@TheMBeat that library is for node.js, NextCloud is running PHP. We can improve the scraper in the RecipeService.php file, if you have suggestions

TheMBeat commented 4 years ago

@mrzapp

That was my mistake, I thought since the frontend might become JavaScript, you could use the scraper.

This PHP based scraper could be helpful then. PHP Recipe Scraper

mrzapp commented 4 years ago

@TheMBeat that project is not updated very frequently, and as it stands, our own scraper supports more websites, because we're relying on the schema.org standard.

As I mentioned, if you have ideas on how to improve our own scraper, then by all means open a new issue or pull request on it. But I don't want to include scraper libraries that only support a limited range of sites, as that would deteriorate the quality of this app significantly.

TheMBeat commented 4 years ago

@mrzapp, thanks for the quick response. I can understand that line of reasoning. It was just an idea. I'll see if I can come up with an improvement that I can submit. By the way, I think this app is excellent.

mrzapp commented 4 years ago

@TheMBeat thank you :) All help is welcome

mrzapp commented 4 years ago

Hey @sam-19, sorry for the long wait, I've been buried in work. I am looking through your changes and hoping to merge them into the master branch by the end of today.

mrzapp commented 4 years ago

@sam-19 @TheMBeat @mcorteel @christianlupus the changes from the 0.7.0 branch have now been merged with master, and I've tested all functionality manually. So far, everything works very well.

As for the practices used in the rewrite, such as writing to the window scope and whether or not to use semicolons, I'll consider those issues trivial compared to getting this version out the door. As long as we have a well-documented and functional code base (which we definitely do, thanks to @sam-19 for that), I believe we can always clean up as needed in later patches.

Please, anyone who has the time, try out the latest changes and report any errors or unexpected behaviour you find. It'd also be great if you could help weed out unused code, should you run into any.

TheMBeat commented 4 years ago

Hello together and sorry for the late reply, first of all, thanks to @sam-19 for his great work. I finally had some time to test and experiment with Vue myself. For testing I added a markdown renderer for the how-to steps and description. image

I didn't find any bugs, but I will continue testing when I get the chance.

I am really looking forward to the release.