Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The open now is currently being calculated server-side. That leads to logical problems with caches like boost or other proxy-caches. The result is cached as-is even when the result should be not open.
A possible solution would be to implement this check in JavaScript dynamically. Any other solutions to this problem?
Comments
Comment #2
AnybodySorry the module we used is 'office hourse', not 'opening hours' - so moving to the right project.
Comment #3
AnybodyI'll now try to solve this via JS because it would be wrong to influence the whole page caching just for the open / closed functionality. A dynamic indicator seems best.
Comment #4
johnvOk. In D8 we are solving this with the new Cache API.
Comment #5
AnybodyI invested 2 hours to port the logic to JavaScript and expose the required date information so that the open / closed functionality is now re-calculated in the client.
I kept the PHP as-is where possible to make the JavaScript degradate gracefully. So the status is still being calculated in PHP for clients who don't support JS. Both status texts are being output into DOM but the non-matching one is hidden by PHP and JavaScript. The JS recalculates the status and hides / shows the current correct status even if the wrong one is being returned from cache.
I ensured that the functionality also works with multiple instances on the same page, for example in views.
Please test a lot and provide feedback. One day I'd be happy to see this as part of the module for perfect behaviour on statically cached pages :)
Patch attached.
Comment #6
AnybodyPatch was missing the office_hours.block.js file. Corrected patch attached!
Comment #7
johnvAh, I already missed this file.
Why do you need an extra js-file?
[EDIT] Oh, I understand. I prefer using then the names office_hours.formatter.js and ~widget.js
Comment #8
johnveffectively, we are moving code from PHP to JS, aren't we?
Comment #9
johnvAnd the same goes for the $next variable from formatter.inc.
Comment #10
johnvI removed the 'open' code from includes/office_hours.formatter.inc. In fact, the 'next day' has the same problem with external caches.
Comment #12
johnvThis is fixed. Thanks.
I guess we need another issue for the 'next day' formatter.
Comment #13
AnybodyThank you very very much! Great to see this in action :)
Is a new stable release planned containing this?
Comment #14
johnvI released 1.9 yesterday.
P.s. JS-files have been renamed and moved into js directory.
If you feel to test the 'open next' formatter, be my guest :-)
Do you agree that this move is not needed in D8, where I have implemented a #cache tag?
Comment #15
AnybodyThank you @johnv. Regarding D8 I'm not sure. If the pages are cached statically the "now open" logic has to be recalculated every time, so it might also be nice to do this in the browser via JS instead. I have not used this in D8 yet.
Comment #16
johnvComment #18
Liam MorlandContent with class element-invisible will be read to users of screen readers, so they will hear both the closed and the open message. It is better to give a class which applies
display: none
since that is hidden from everyone.Comment #19
johnvComment #20
johnvComment #21
AnybodyI'll create a patch for #18. Furthermore I saw that the script seems buggy, we have a case where it shows close while it should be open. Please help to correct this. The JS code was based on the former php code.
Comment #22
Anybody@johnv: Ok I finally found the reason and solution. The problem was that the JS calculation was not compatible with the "group" / "compress" options and because it needs the raw days array for calculation.
Furthermore I fixed the accessibility bug.
Could you please review and commit, because currently the indicator is broken!
Comment #23
AnybodyPS: Could you enable automated testing for this branch and issues perhaps?
That would help to see if the patch applies correctly. Thanks!
Comment #24
johnvI dont know what I did, but the automated testing is now enabled weekly. Is it ok like this?
Comment #25
AnybodyHi @johnv, thank you! Please select "Run on commit and for issues" instead. I suggest in combination with "Stable" core.
Comment #26
AnybodyComment #27
johnvDone.
Comment #29
johnvCommitted . Thanks. I renamed a variable.
Please re-open if there is still a problem.
Comment #30
AnybodyGreat, thank you!!