plenti-themes / compendium

https://plenti-themes.github.io/compendium/
Apache License 2.0
3 stars 1 forks source link

Replacement pattern to slugify category/tag names for their routes #3

Closed jamestagal closed 2 years ago

jamestagal commented 2 years ago

hi @roobyz How are you ?

I have a feature request. I might have mentioned this one when we met. But I think it is a feature worth pursuing. At present the routes for both Categories and Tags are given numbers however for SEO purposes it would be really nice to replicate the Category and Tag names and slugify them to their corresponding routes. I know i was able to do this for Posts by using the replacement pattern fields(title) on the Posts route. Thanks @jimafisk for that help.

Would you be interested to look at implementing this feature?

BTW. I did just try changing the {page} property in post_meta.svelte see below to {name}without any success. I was just tossing a coin to see if it would work :)

   <a href="catgs/{page}" class="meta ml-0.5">
          {name}

Let us know your thoughts.

Best regards, Ben

roobyz commented 2 years ago

hi @jamestagal

I believe that there is still a limitation with the pager functionality that @jimafisk needs to address. Currently an object for a tag looks something like:

{
"pager": 1,
"path": "tags/1",
"type": "tags",
"filename": "tags.json",
"fields": {}
}

You would see one object like that for each tag, where the pager and path values would increment by 1 for each tag. Because fields contains an empty object, fields(title) would return undefined. To use fields(title), we need to see is something more like:

{
"pager": 1,
"path": "tags/1",
"type": "tags",
"filename": "tags.json",
"fields": { "title": "Science"}
}

In addition, if we had 30 posts with the "Science" tag, we would ideally have multiple pages for "Science" so that we might want to see something more like:

{
"pager": 1,
"path": "tags/science/1",
"type": "tags",
"filename": "tags.json",
"fields": { "title": "Science"}
}

I did provide a recommendation last week to Jim, but I'm thinking this would take a little work to complete. ;-) Not sure since I'm still waiting to hear back.

Cheers, Roberto

jimafisk commented 2 years ago

Currently plenti works entirely off file system routing. Your content folder can be used for page endpoints, but also for menu links, taxonomy lists, or other information that may or may not ultimately have a route. Basically content is for things you'd want an end user to be able to change (the git-cms we're working on should make this possible) and your layouts are for developers.

I've implemented a couple of small updates to demonstrate how you could use the tag name instead of a paginated route: https://github.com/plenti-themes/compendium/pull/4

Screenshot ![tags](https://user-images.githubusercontent.com/5913244/154526370-9305b5ec-08e4-4996-ab6f-14ddf6dd133c.png)

In the future it would be cool to implement dynamic routing from your client app, you actually could plenti eject your router.svelte file and implement this right now if you wanted to. Will something like the example above work in the meantime?

roobyz commented 2 years ago

hi @jimafisk

If I understand correctly, your change would alter my approach of "dynamically generating" tags based on what is set in the posts, to an approach where we pre-design a set of tags using content files. We would then create one content file for each tag that we would want. Later, when we assign those tags to the posts, they need to be assigned from the list of tag content files that we created in advance.

Correct?

Roberto

jimafisk commented 2 years ago

Yeah that's correct, totally understand if that's not aligned with your vision for the theme. The content/tags folder would become a defined taxonomy that you could reference in the posts. The client router we use, Navaid, does support wildcard routes, so we could probably create something more dynamic.

roobyz commented 2 years ago

Ok... it is different than I envisioned it, but I'll give it a go. There are a few more required changes to make it work, but I can hash those out.

jimafisk commented 2 years ago

If it makes you unhappy don't do it! I'm sure we can find a solution that you like better, probably with wildcard routes, it just might not happen instantly :).

roobyz commented 2 years ago

hi @jimafisk... I've pushed some updates to the issue-3 branch following on your approach. The logic works however, I haven't figured out how the get the {#key tag} to function correctly with my changes. Thoughts?

roobyz commented 2 years ago

@jimafisk ... I figured out a solution. Should push out a draft soonish. ;-)

roobyz commented 2 years ago

@jamestagal merged your feature request. It required a bit of refactoring.

jamestagal commented 2 years ago

Hi @roobyz

I was clicking around the site after your refactor and saw an catgs/undefined(https://plenti-themes.github.io/compendium/catgs/undefined) error.

So If you click on the Design tag on this post page you get that error. In fact I get the error when clicking on any Category listed on any post page including the featured pages.

Cheers, Ben

roobyz commented 2 years ago

Good catch @jamestagal... I forgot to update the post_meta component!

roobyz commented 2 years ago

Just pushed the refactored post_meta component @jamestagal

roobyz commented 2 years ago

@jamestagal... be mindful of the content changes to make the tags and categories work. The category and tags have two new keys: name and route. Also, I added the route key to the projs.json content so that you can specify the route you want to see.

jamestagal commented 2 years ago

Thank you @roobyz I really appreciate your work on this and can see the amount of refactoring it required. And I admire your lightning pace to code these change!!! because it usually takes me days to implement your changes into my code after a refactor!!! hahahaha...But on the plus side, I am becoming an expert in the art of refactoring my code:) But seriously you are a champion.

Thanks again. Ben

roobyz commented 2 years ago

How is the refactoring going @jamestagal?

jamestagal commented 2 years ago

Hi @roobyz To be honest, I haven't had a chance to start yet!!! I have been so busy with work and not had the energy in the evenings to attempt. But I do plan to start tonight :) Thanks for checking in! I will let you know how it goes :)

Cheers, Ben

jamestagal commented 2 years ago

Hi @roobyz

After refactoring my code, I am getting an error. I can see reference to scripts_catgs_tags_svelte_catgs_tags

javascript stack trace: TypeError: Cannot convert undefined or null to object
    at Function.values (<anonymous>)
    at layouts_scripts_catgs_tags_svelte_catgs_tags (create_ssr:58:18)
    at create_ssr:31:14
    at $$render (create_ssr:1555:22)
    at Object.render (create_ssr:1563:26)
    at create_ssr:1:58 (File: /home/runner/work/plenti/plenti/cmd/build.go, Line: 140)
Total build took 1.967516625s

Could you see where I might have done something wrong? Hi @roobyz
I have been over and over the code from your latest commits and my refactor and I can't see why or where the issue is. If I push my local files to another branch would you mind taking a look for me pls?

Cheers, Ben

jamestagal commented 2 years ago

hello @roobyz I figured out how to create a new branch on my repo and I committed and pushed all the changes I made and your commits from 20th Feb. https://github.com/jamestagal/edtechdesigner/tree/refactorFeb20

Please have a look when you get a chance. Best regards, Ben

UPDATE Hi @jimafisk
I haven't heard anything from Roberto since the last issue I had. See above. I hope everything is ok with him. If you get a chance would you mind having a look at the error message and see if you could suggest a course of action? I would appreciate it.. best regards, Ben

roobyz commented 2 years ago

hi @jamestagal, Sorry for the delayed response. I did a plenti build and did not get any errors. Could you summarize any issues or challenges that you have in the branch?

There is a file layouts\scripts\catg_tags.svelte which is likely related to the issue with the error message from your prior post.

Thanks, Roberto

jamestagal commented 2 years ago

Hi @roobyz How you are? At that time, I just couldn't get it to build and got the error message listed above.. but I will try runningplenti build again since updating Planti to v0.5, and see what happens. I'll get back to you tonight. Thanks Ben

roobyz commented 2 years ago

Hi @jamestagal

I'm feeling well. I was a bit under the weather for a few months and also overworked.

So... I figured out that after updating your repo, and then changing to your branch, I needed to run plenti theme update compendium before I could see the issue you were having. Then I realized that you were missing an exclude in your plenti.json file. Like:

{
    "build": "public",
    "baseurl": "/",
    "theme": "compendium",
    "theme_config": {
        "compendium": {
            "url": "https://github.com/plenti-themes/compendium",
            "commit": "ea420052c6dd1c7806ed6009f8253841301d7b3a",
            "exclude": ["content", "assets"]
        }
    },
    "local": {
        "port": 3000
    },
    "routes": {
        "404": ":filename",
        "catgs": ":fields(route)",
        "index": ":paginate(totalPages)",
        "pages": "pages/:filename",
        "posts": "post/:fields(title)",
        "projs": ":fields(route)/:paginate(totalProjPages)",
        "tags": ":fields(route)"
    },
    "cms": {
        "repo": "",
        "redirect_url": "",
        "app_id": "",
        "branch": ""
    }
}

This would exclude the Content in my theme from needing to render with your content, HOWEVER... I believe there is a bug (@jimafisk) in plenti which prevents your index.json file from working with that exclude applied. So... what I did was remove the tag and catg content from my theme folder (themes/compendium/content/catgs and themes/compendium/content/tags) and it resolved the issue.

Basically it was failing on my "random.json" example content, which you obviously didn't have, and the category/tag logic was choking on that and then dying. ;-)

Roberto

jamestagal commented 2 years ago

Hi @roobyz

I tried what you outlined above to that branch except for running plenti theme update compendium because that should be up to date! So added the exclude properties and removed those .json files. See screenshot below.

Screen Shot 2022-04-05 at 10 06 10 pm

Then I got the following error in the terminal:

███████▒▒▒ Building... 2022/04/05 21:59:41 errs.go:57: Could not create props: ReferenceError: layouts_content_index_svelte is not defined /home/runner/work/plenti/plenti/cmd/build/data_source.go on line 344

javascript stack trace: ReferenceError: layouts_content_index_svelte is not defined
    at create_ssr:1:1547 (File: /home/runner/work/plenti/plenti/cmd/build.go, Line: 140)
Total build took 1.891863542s

Serving site from your "public" directory.
Visit your site at http://localhost:3000/

So I'm not sure what i am doing differently!

Ben

roobyz commented 2 years ago

hi @jamestagal Ahhh... sorry if I wasn't clear.

Please:

  1. remove the exclude property that should be there, because it is triggering the bug that I mentioned. ;-)
    • I did look at your index.json and it seems to be equivalent to the original
    • it looks like the layouts_content_index_svelte is failing because index.json gets excluded from both places (project and theme) when the exclude is applied
  2. remove the sample tag/catg json files from my theme folder (i.e. you want to keep ONLY the json files with the category and tags that you created and not the samples from my theme edtechdesigner/themes/compendium/content)
    • this shouldn't be required with the exclude applied
    • at some point, when the plenti bug is resolved, we should add back the exclude to the plenti.json and skip this step going forward

Roberto

jamestagal commented 2 years ago

Hi @roobyz Thanks that worked :) Sorry I didn't understand you correctly the first time. Thank you :)

Though, I noticed I am still getting a pagination error ...I think this is an older issue that I didn't resolve...It happens when I navigate to the next page from Posts..and when I count my total posts, there are two posts that aren't listed on the second page just the error page.

The default Posts page get this url http://localhost:3000/posts while the second page gets this url error: http://localhost:3000/undefinedposts/2

Pls let me if you know what might be happening.

Thanks again for you help.

Ben

roobyz commented 2 years ago

hi @jamestagal

Ahh... I thought you figured that out already, since it was marked as "Closed". :-)

Your projs.svelte is missing the baseurl property on the Pagination component line, which results in the "undefined" being prepended to the route. It should look like:

            <Pagination {content} {currentPage} {totalPages} {baseurl} />

Cheers, Roberto

jimafisk commented 2 years ago

Hi @roobyz, thanks for flagging the exclude bug to me. Is there an ticket in the issue queue for this? Is it not excluding what you're specifying or is it throwing errors? Using that directive can be somewhat dangerous if not accounted for in the theme. I'd be happy to take a look to see if I can fix it.

roobyz commented 2 years ago

hi @jimafisk

I just opened an issue: https://github.com/plentico/plenti/issues/186

Cheers, Roberto

jamestagal commented 2 years ago

Thank you @roobyz I see the missing code and that did the trick!

So should I be merging this branch back into my Main branch? I haven't done that before...:) but i will give it a try!

Thanks again for all your help. I really appreciate it

Regards, Ben

roobyz commented 2 years ago

Sounds like a good plan @jamestagal. Pretty easy to do in GitHub. Just open a pull request and then merge and afterward delete the branch. You'll then need to pull back the merged repo and delete the branch from your local repo.

jamestagal commented 2 years ago

Ok thanks @roobyz

Something like this? Screen Shot 2022-04-06 at 11 11 43 am

Just on the last point...how would I go about deleting that branch from my local repo?

Ben

roobyz commented 2 years ago

Yes... and you can already see that you are able to merge. After opening the pull request, you'll get the option to merge and add more notes. And when you merge, you'll get the option to remove the branch.

jamestagal commented 2 years ago

@roobyz like this? Screen Shot 2022-04-06 at 11 31 18 am

and then, Screen Shot 2022-04-06 at 11 40 16 am

jamestagal commented 2 years ago

@roobyz Now do I do a git pull on the main branch from my local machine to get the latest version on Github? I notice that when I type git status in VS code it says "Your branch is up to date with 'origin/main'! I though it might have automatically recognised that the main branch on Github has now changed!

Ben

roobyz commented 2 years ago

@jamestagal... you should now run git checkout main locally (aka switch to your main branch)... then run git pull to update your main branch. After that you can delete your merged branch locally... git branch -d refactorFeb20. At that point you should be synchronized with GitHub.

What is important to note about Git is that it is a "distributed version control" which means that your local Git and GitHub can both stand alone and operate separately. So if you want them aligned, you need to synchronize them. This is one reason why Git is so powerful and flexible.

I also have clone of your Git repo locally which doesn't have all the changes you made or the merge. For me, I could synchronize to GitHub or I could delete my local version and then clone again. With either approach I would then end up with a synchronized version of your changes. ;-)

jamestagal commented 2 years ago

thanks @roobyz ...yes Git is really powerful and has been quite a learning curve for me... but I appreciate your detailed explanation... I think I'm getting close to completing the customisations I planned to do with your theme... only small things now such as changing the font and switching the tags and catg for my articleSnapshot ... best regards Ben

jamestagal commented 2 years ago

hi @roobyz After running a git pull and git branch -d refactorFeb20 and running plenti serve I am getting the following error:

████▒▒▒▒▒ Building... 2022/04/06 15:27:49 errs.go:57: Can't render htmlComponent: TypeError: Cannot read property 'filter' of undefined /home/runner/work/plenti/plenti/cmd/build/data_source.go on line 349

javascript stack trace: TypeError: Cannot read property 'filter' of undefined
    at layouts_scripts_catgs_tags_svelte_catgs_tags (create_ssr:8:27)
    at create_ssr:31:14
    at $$render (create_ssr:1555:22)
    at Object.render (create_ssr:1563:26)
    at create_ssr:1:58 (File: /home/runner/work/plenti/plenti/cmd/build.go, Line: 140)
Total build took 2.343452167s

Do you think i need to run a build command or do something else? Ben

roobyz commented 2 years ago

Hi @jamestagal

Did you add html.svelte to your layouts/global/ folder? The original branch did not have that file and if you compare to the version in my theme folder, you see that your version is missing a prameter in the catgs_tags function call. It should look as follows:

  let ctObj = catgs_tags(allPosts, allContent);

I recommend that your remove the file from your layouts/global/ folder, since the only difference is the missing parameter. This resolves your issue.

Roberto

jamestagal commented 2 years ago

Hi @roobyz Yes that worked.....thank you so much.. I have to be more careful when copying these files over and make sure they reflect the theme.

Thanks again.