Pechente / kirby-admin-bar

Adds an admin bar to the frontend in Kirby CMS so you can easily edit the current page or access common panel pages
MIT License
26 stars 0 forks source link

Pages Cache #1

Open bnomei opened 2 days ago

bnomei commented 2 days ago

love your plugin. way nicer than my hacks.

Doing a check for user like that will disable the pages cache for said page. Thus your plugin is currently incompatible with the pages cache and you might want to put a warning in the readme.

https://github.com/Pechente/kirby-admin-bar/blob/e01fccf2cc7ed535ff36aab9ac417a8da1c40620/snippets/admin-bar.php#L5

a solution would be to instead of using a check in PHP first query an endpoint and use js to show the area. in your case hydrate with user information like image and name as well. like this with alpine js

snippet

<?php // checking for user or setting a cookie would block pages cache, thus use an api call to check
if (isset($page)) {
    $url = $page->panel()->url();
}
$show = $show ?? false;
?>
<!-- edit -->
<div class="fixed bottom-0 right-0"
     x-data="{ show: <?php e($show, 'true', 'false') ?> }"
     x-init="response = await fetch('<?= site()->url() ?>/edit'); show = response.status === 200"
     <?php e(!$show, 'x-show="show"') ?> <?php e($show, '', 'style="display: none"') ?>>
  <a class="block p-2 lg:p-4 bg-darkgray text-white hover:bg-gray hover:text-darkgray rounded-tl" href="<?= $url ?>" rel="nofollow noindex">
    <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32" class="w-8 h-8 lg:w-12 lg:h-12"><g fill="currentColor" class="nc-icon-wrapper"><path d="M26,31H2a1,1,0,0,1-1-1V6A1,1,0,0,1,2,5H14V7H3V29H25V18h2V30A1,1,0,0,1,26,31Z"></path> <path data-color="color-2" d="M21.086,5.5l-9.793,9.793a1,1,0,0,0-.241.391l-2,6A1,1,0,0,0,10,23a.987.987,0,0,0,.316-.052l6-2a1,1,0,0,0,.391-.241L26.5,10.914Z"></path> <path data-color="color-2" d="M30.707,5.293l-4-4a1,1,0,0,0-1.414,0L22.5,4.086,27.914,9.5l2.793-2.793A1,1,0,0,0,30.707,5.293Z"></path></g></svg>
  </a>
</div>

route

[
            'pattern' => 'edit',
            'language' => '*',
            'action' => function () {
                return \Kirby\Cms\Response::json(code: kirby()->user() !== null ? 200 : 204);
            }
        ],
Pechente commented 2 days ago

Thank you for the feedback! I didn't even consider the pages cache. I will add the warning right away and implement a fix tomorrow

lukasbestle commented 2 days ago

Chiming in here quickly: Calling $kirby->user() will not completely disable the pages cache, but it will trigger the automatic cacheability detection. For the plugin this should mean:

A note on Bruno's proposed solution: Maybe it could be implemented so that the route either returns the rendered admin bar snippet for the user (thus avoiding having to put the HTML into all responses, even for visitors) or an empty response with 204 status code. The frontend part can then just either inject that HTML into the page as returned from the backend or (if 204 response) do nothing. This should be doable without JS libraries.

Pechente commented 2 days ago

This should be doable without JS libraries.

This was my plan. To keep it as lean as possible. However an additional request means that the page might have some unpleasant jumps since we do not know in advance whether the admin bar should be rendered or not. The current solution is also very clean in a way that guests won't even know it's there at all - which I quite like. I will do some testing tomorrow and see if there's a decent compromise for this.

lukasbestle commented 2 days ago

Absolutely, that was also a thought I had. Maybe it could be an option so those who don't use the pages cache, Staticache or a CDN don't get the added JS and fetch request but the snippet directly.

SebastianEberlein-JUNO commented 1 day ago

We have a plugin that inserts a snippet into the HTML in a page.render:after hook. But only if the user is logged in and without interfering with the Kirby cache: https://github.com/junohamburg/kirby-reload-on-save?tab=readme-ov-file#setup

This might work for your plugin as well.

Pechente commented 1 day ago

Thanks a lot for the recommendation! That is a really good solution. I will implement it later today.

bnomei commented 1 day ago

Kirby cache

This should only be true for the basic "Pages Cache" via PHP but not apply to the pages cache with the additional Staticache plugin. As the staticache will have the server render the HTML file directly without booting Kirby at all.

But non the the less a neat solution to use page.render:after 👏 .

Pechente commented 1 day ago

Thanks everyone for the feedback! I changed the implementation to use the hook instead of the snippet and released it as a 2.0 to avoid breaking people's current deployments.