Jump to content

Recommended Posts

Thanks so much @d'Hinnisdaël, really appreciate the effort here 🙂 Will review, test and merge this morning. Regarding videos, you might actually want to hold off on that for a bit, as I'd first like to add the additional sub-element class for it, and perhaps a few others as well. The Sitemap class itself comes from ThePixelDeveloper over at GitHub, and so I think I might want to upgrade that instead, to utilise the newer features / code-base.

Share this post


Link to post
Share on other sites

So I've been a bit busy this morning refactoring and brooming. A man is busy of late, but hey, gotta make some time for these things eventually.

Anyways, couple of things I've done so far:

  1. Updated the underlying sitemap package.
  2. Refactor: split some code out into concerns, added support helpers, added return types and fixed up doc blocks.
  3. Dropped support for PW 2.8 and PHP < 7.1 (should be 7.2, but fine for now)
  4. Manually pulled in the merge request with an additional static method to add the language support hooks. I don't really have a good test case to work with, but this is the idea:
use MarkupSitemap;

wire()->addHookAfter(MarkupSitemap::getAdditionalPages, function ($event) {
  $page = $event->arguments(0);
  $language = $event->arguments(1);

  MarkupSitemap::applyLanguageSupportHooks();

  // Add additional pages
});

It should internally track whether or not they've been added, but I've simply not tested it yet, so can't say for sure. 😅That aside though, surely I can add the hooks directly in ___getAdditionalPages?

If you'd like to give it a spin, please check out the develop branch directly.

  • Like 2

Share this post


Link to post
Share on other sites
3 minutes ago, Mike Rockett said:

That aside though, surely I can add the hooks directly in ___getAdditionalPages?

I actually see I did that already 😅So feel free to leave out the static method call above and let me know if it works.

  • Like 2

Share this post


Link to post
Share on other sites
4 hours ago, Mike Rockett said:

If you'd like to give it a spin, please check out the develop branch directly.

Just gave it a try — works like a charm! Thanks for merging so quickly.

A thought on the hook name: would it make more sense to call it getAdditionalPageUrls (including "urls") to clarify it's returning additional URLs for a specific page? One might need a hook at some point to get additional pages independent of any other page (who knows why), which would then appropriately be called getAdditionalPages.

  • Like 1

Share this post


Link to post
Share on other sites
1 hour ago, d'Hinnisdaël said:

Just gave it a try — works like a charm! Thanks for merging so quickly.

A thought on the hook name: would it make more sense to call it getAdditionalPageUrls (including "urls") to clarify it's returning additional URLs for a specific page? One might need a hook at some point to get additional pages independent of any other page (who knows why), which would then appropriately be called getAdditionalPages.

Great stuff - and does the language support hook-setup work as expected?

You're quite right indeed, will sort that out some time tomorrow. 🙂

Share this post


Link to post
Share on other sites
5 minutes ago, Mike Rockett said:

Great stuff - and does the language support hook-setup work as expected?

Yes, working as intended!

  • Like 1

Share this post


Link to post
Share on other sites

Hi @Mike Rockett

I would like to notice you that the php requirements for this module is incorrect. Since you use 

338 public function canBeIncluded(Page $page, ?array $options): bool

you need php 7.1 at least

Share this post


Link to post
Share on other sites

@nabo - Thanks for letting me know. I see I updated composer.json and not info.json as well.

Update: I have made the change in the repo, but will only be able to update the module directory when I find the password.

  • Like 1

Share this post


Link to post
Share on other sites

@Mike Rockett, how do you feel about preventing caching — entirely, or as a config setting — for logged in users? I think it's probably a bit unexpected that sitemap can include pages that guests have no access to 🙂

In cases where sitemap is cached for logged in users, cache key should probably include the roles of the user. That won't be entirely bulletproof, but at least less likely to cause problems 🤔

  • Like 3

Share this post


Link to post
Share on other sites

@teppo, interesting idea indeed. In my opinion though, I feel like that defeats the purpose of a sitemap, which is really designed for guest and search engine access (in which case, only templates of those pages should be added to the config in the first place). I don't see why items should be conditionally added based on role if the consumer won't make much use of it… If you can see a reason how/why they would, then I'm happy to add it in.

Share this post


Link to post
Share on other sites

@Mike Rockett, I think we're talking about different things here. Sorry, my bad — should've been more clear! 🙂

The thing is that currently this module caches content for logged in users as well, which in turn means that the XML sitemap that guest users then view might contain non-public content. This probably shouldn't be the case, at least not by default: if the user is logged in, it would be safer not to cache anything at all, or you may end up with a sitemap that publicly displays content that wasn't meant to be public (even if only URLs and update times)... not to mention that (as you pointed out above) including non publicly viewable URLs is not particularly useful anyway.

The idea about including roles in cache key was mostly just so that if there's indeed some use case where folks prefer current behaviour, that could still work. I can't think of many reasons for that either, though, unless it's for some sort of an internal API / index or something 🙂

Share this post


Link to post
Share on other sites

@teppo – fair enough, you're actually quite right. So the options are then to either have the option to turn off caching for authed users or use use role-based caching for authed users, or perhaps even dictate some more fine grained rules behind which pages can be added to the sitemap. At the end of the day, the sitemap should always resemble publicly-viewable content only, so perhaps that's a better option (though I don't know how feasible it is from an implementation perspective). Another thing to note is that individual pages can be turned off from their Sitemap settings, though perhaps in some cases, that could be quite a chore (depending on the tree-structure and whether or not said pages are all common to one parent).

Share this post


Link to post
Share on other sites
1 hour ago, Mike Rockett said:

So the options are then to either have the option to turn off caching for authed users [...]

I think this would be a very good first step, and would probably be quite enough for almost all use cases. If you could make this change, I'd be a very happy user indeed 🙂

1 hour ago, Mike Rockett said:

Another thing to note is that individual pages can be turned off from their Sitemap settings, though perhaps in some cases, that could be quite a chore (depending on the tree-structure and whether or not said pages are all common to one parent).

Options like these are great to have, but from my point of view automation is always the best solution (if it can be done, that is). After realizing that the cache can now contain pages that are not public I turned to this option, but doing it manually on a site with loads of non-public pages all over the place... not fun 😅

Slightly off-topic:

I was under the impression that the "Templates without sitemap access" setting would also affect child pages, removing them from the sitemap, but at least on one site it doesn't work like that: parent page (the one with a template that was specifically selected here) doesn't show up, but its children still do. Not sure if that's a bug or something I've misunderstood?

Share this post


Link to post
Share on other sites

FYI, I include an "ignore" field to my templates that allows for control over a page's visibility in menus, search results, and the sitemap. Then, I add that logic to the appropriate template files:

image.png.23e6501fad13408fd0685ac54059f9ee.png

// Hide from XML sitemap
case ($page->hasField("ignore") && $page->ignore->hasValue("sitemap_ignore")):
	return "";
	break;

// Hide child pages from XML sitemap
case ($page->parent->hasField("ignore") && $page->parent->ignore->hasValue("sitemap_ignore_children")):
	return "";
	break;

Here is the exported field in case anyone else wants to do the same:

{
    "ignore": {
        "id": 392,
        "name": "ignore",
        "label": "Visibility",
        "flags": 0,
        "type": "FieldtypeOptions",
        "inputfieldClass": "InputfieldCheckboxes",
        "collapsed": 2,
        "optionColumns": 0,
        "tags": "meta",
        "icon": "ban",
        "initValue": "",
        "showIf": "",
        "themeOffset": "",
        "themeBorder": "",
        "themeColor": "",
        "columnWidth": 100,
        "required": "",
        "requiredIf": "",
        "defaultValue": "",
        "export_options": {
            "default": "1=menu_ignore|Hide page from navigation menus\n2=menu_ignore_children|Hide child pages from navigation menus\n3=search_ignore|Hide page from site search results\n4=search_ignore_children|Hide child pages from site search results\n5=sitemap_ignore|Hide page from XML sitemap\n6=sitemap_ignore_children|Hide child pages from XML sitemap"
        }
    }
}

 

Share this post


Link to post
Share on other sites

@teppo – I've pushed an update to the develop branch that adds an update policy config option. I think the sensible default is to set it to guests only, which is what I've done here. Let me know if you feel it should be the other way around. The HTTP headers now also provide a reason for skipping the cache. Would you be happy to test it out?

Regarding templates without sitemap access, the idea is to only target pages. Built it way back when, so can't really remember why I did it this way, but I do remember having a reason. We could well add an option to also say that children of these templates should be excluded... 🤔

--

There are other issues on the repo at the moment, some of which discuss features people would like to see. Unfortunately, my time is very limited and so I invite module developers to contribute if they feel up to it (I'd be super grateful). Happy to spend time discussing things and reviewing PRs and issuing quick bug fixes, but am not able to get to feature development on this module right now…

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

  • Recently Browsing   0 members

    No registered users viewing this page.

  • Similar Content

    • By neophron
      Hi guys,
      after getting a complain message from google about a robots.txt (where everything is ok), I searched for an online tool, where I can test my robots.txt files. I found this website: https://technicalseo.com/tools/
      This page offers a bunch of nice tools, just wanted to share it with you.

    • By MateThemes
      Hello everyone!
      I have a markup and image field question:
      I have a markup in which a gallery preview should be displayed on home page. This gallery have a special markup, 3 of 4 images have special image ratio.

      The gallery looks like this. 
      The html markup is like this:
      <div class="uk-section-muted" uk-scrollspy="target: [uk-scrollspy-class]; cls: uk-animation-slide-top-small; delay: 200;"> <div data-src="./assets/images/arrahof/home-restaurant-bg.svg" class="uk-background-norepeat uk-background-contain uk-background-top-center uk-section uk-section-large" uk-img> <div class="uk-container uk-container-small"> <div class="uk-margin-large" uk-grid> <div class="uk-width-1-1@m"> <h2 class="uk-text-center" uk-scrollspy-class>Das Angebot im ArraHof</h2> </div> </div> <div class="uk-grid-small uk-grid-margin-small" uk-grid> <div class="uk-width-expand@s"> <div class="uk-margin-remove-vertical uk-text-center" uk-scrollspy-class> <a class="el-container uk-inline-clip uk-transition-toggle uk-link-reset" href="#"> <img class="el-image" data-src="https://via.placeholder.com/610x604" data-sizes="(min-width: 610px) 610px" data-width="610" data-height="604" alt="Placeholder Image" uk-img> <div class="uk-overlay-default uk-transition-fade uk-position-cover"></div> <div class="uk-position-center uk-position-small"> <div class="uk-overlay uk-padding-large uk-transition-fade uk-margin-remove-first-child"> <h3 class="el-title uk-h4 uk-heading-divider uk-transition-slide-top-small uk-margin-top uk-margin-remove-bottom">Lorem Ipsum</h3> <div class="el-content uk-panel uk-transition-slide-bottom-small uk-margin-top">Comfort Food</div> </div> </div> </a> </div> </div> <div class="uk-width-expand@s"> <div class="uk-margin-remove-vertical uk-text-center" uk-scrollspy-class> <a class="el-container uk-inline-clip uk-transition-toggle uk-link-reset" href="#"> <img class="el-image" data-src="https://via.placeholder.com/610x604" data-sizes="(min-width: 610px) 610px" data-width="610" data-height="604" alt="Placeholder Image" uk-img> <div class="uk-overlay-default uk-transition-fade uk-position-cover"></div> <div class="uk-position-center uk-position-small"> <div class="uk-overlay uk-transition-fade uk-margin-remove-first-child"> <h3 class="el-title uk-h4 uk-heading-divider uk-transition-slide-top-small uk-margin-top uk-margin-remove-bottom">The Williams family</h3> <div class="el-content uk-panel uk-transition-slide-bottom-small uk-margin-top">Comfort Food</div> </div> </div> </a> </div> </div> </div> <div class="uk-grid-small uk-grid-margin-small" uk-grid uk-height-match="target: .uk-card; row: false"> <div class="uk-width-2-3@s"> <div class="uk-margin-remove-vertical uk-text-center" uk-scrollspy-class> <a class="el-container uk-inline-clip uk-transition-toggle uk-link-reset" href="#"> <img class="el-image" data-src="https://via.placeholder.com/610x400" data-width="610" data-height="400" alt="Placeholder Image" uk-img> <div class="uk-overlay-default uk-transition-fade uk-position-cover"></div> <div class="uk-position-center uk-position-small"> <div class="uk-overlay uk-transition-fade uk-margin-remove-first-child"> <h3 class="el-title uk-h4 uk-heading-divider uk-transition-slide-top-small uk-margin-top uk-margin-remove-bottom">The Williams family</h3> <div class="el-content uk-panel uk-transition-slide-bottom-small uk-margin-top">Comfort Food</div> </div> </div> </div> </a> </div> <div class="uk-width-expand@s"> <div class="uk-margin-remove-vertical uk-text-center" uk-scrollspy-class> <a class="el-container uk-inline-clip uk-transition-toggle uk-link-reset" href="#"> <img class="el-image" data-src="https://via.placeholder.com/610x820" data-sizes="(min-width: 610px) 610px" data-width="610" data-height="820" alt="Placeholder Image" uk-img> <div class="uk-overlay-default uk-transition-fade uk-position-cover"></div> <div class="uk-position-center uk-position-small"> <div class="uk-overlay uk-transition-fade uk-margin-remove-first-child"> <h3 class="el-title uk-h4 uk-heading-divider uk-transition-slide-top-small uk-margin-top uk-margin-remove-bottom">The Williams family</h3> <div class="el-content uk-panel uk-transition-slide-bottom-small uk-margin-top">Comfort Food</div> </div> </div> </div> </a> </div> </div> </div> <div class="uk-margin-large" uk-grid> <div class="uk-width-1-1@m"> <div class="uk-text-lead uk-width-xxlarge uk-margin-auto uk-text-center" uk-scrollspy-class>Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur.</div> <div class="uk-margin-medium uk-text-center" uk-scrollspy-class> <a class="el-content uk-button uk-button-default uk-button-large" href="#">Unterkunft</a> </div> </div> </div> </div> </div> How can I achieve when I have an image field to add the custom markup within the image field? I have no clue to get this done.
      Thank you!
    • By NehaPillai
      Hello Everyone, I was trying to update SEO meta title, description and meta keywords for my website in Process Wire CMS but it saving in the backend but it is not reflecting on my website, Please help me regarding this error. Please find below attached screen shot for your ref. TIA.


    • By stanoliver
      My aim is to output a very basic xml document which should be styled with a few css-styles.
      <?xml version = "1.0"?> <contact-info> <name>Donal Duck</name> <company>Superducks</company> <phone>(011) 123-4567</phone> </contact-info> How do I implement it with processwire?
    • By franciccio-ITALIANO
      Hi, we can choose the "headline" and "title" and "summery" in panel page of processwire, but we can't write the "metadecriptions" and "tags".
       I can write mdescropt and tags in templates, but I've same templates for many articles... so, how I can change mdescription and tags?

      Thanks...
×
×
  • Create New...