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?

CommentFileSizeAuthor
#22 dynamically_calculate_open_closed_status_for_cached_pages-2919519-22.patch4.17 KBanybody
#6 dynamically_calculate_open_closed_status_for_cached_pages-2919519-6.patch8.86 KBanybody
#5 dynamically_calculate_open_closed_status_for_cached_pages-2919519-5.patch8.1 KBanybody

Comments

Anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Project: Opening hours » Office Hours

Sorry the module we used is 'office hourse', not 'opening hours' - so moving to the right project.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

I'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.

johnv’s picture

johnv
Primary language Dutch
commented

Ok. In D8 we are solving this with the new Cache API.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Active » Needs review
StatusFileSize
new dynamically_calculate_open_closed_status_for_cached_pages-2919519-5.patch8.1 KB

I 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.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
StatusFileSize
new dynamically_calculate_open_closed_status_for_cached_pages-2919519-6.patch8.86 KB

Patch was missing the office_hours.block.js file. Corrected patch attached!

johnv’s picture

johnv
Primary language Dutch
commented

Ah, 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

johnv’s picture

johnv
Primary language Dutch
commented

effectively, we are moving code from PHP to JS, aren't we?

johnv’s picture

johnv
Primary language Dutch
commented

And the same goes for the $next variable from formatter.inc.

johnv’s picture

johnv
Primary language Dutch
commented

I removed the 'open' code from includes/office_hours.formatter.inc. In fact, the 'next day' has the same problem with external caches.

  • johnv committed 2b1a7a4 on 7.x-1.x authored by Anybody 
    Issue #2919519 by Anybody: 'Open now' formatter fails with boost /...
johnv’s picture

johnv
Primary language Dutch
commented
Title: "Open now" fails with boost / caches » "Open now" formatter fails with boost / caches
Version: 7.x-1.x-dev » 7.x-1.8
Status: Needs review » Fixed

This is fixed. Thanks.
I guess we need another issue for the 'next day' formatter.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thank you very very much! Great to see this in action :)
Is a new stable release planned containing this?

johnv’s picture

johnv
Primary language Dutch
commented

I 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?

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thank 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.

johnv’s picture

johnv
Primary language Dutch
commented

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

Content 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.

johnv’s picture

johnv
Primary language Dutch
commented
Status: Closed (fixed) » Active
johnv’s picture

johnv
Primary language Dutch
commented
Component: Code » Code - formatter
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Active » Needs work

I'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.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new dynamically_calculate_open_closed_status_for_cached_pages-2919519-22.patch4.17 KB

@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!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes

PS: Could you enable automated testing for this branch and issues perhaps?
That would help to see if the patch applies correctly. Thanks!

johnv’s picture

johnv
Primary language Dutch
commented

I dont know what I did, but the automated testing is now enabled weekly. Is it ok like this?

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Hi @johnv, thank you! Please select "Run on commit and for issues" instead. I suggest in combination with "Stable" core.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Version: 7.x-1.8 » 7.x-1.x-dev
johnv’s picture

johnv
Primary language Dutch
commented

Done.

  • johnv committed 026eeae on 7.x-1.x authored by Anybody 
    Issue #2919519 by Anybody, johnv: OpenNow formatter fails with boost /...
johnv’s picture

johnv
Primary language Dutch
commented
Version: 7.x-1.x-dev » 7.x-1.9
Status: Needs review » Fixed

Committed . Thanks. I renamed a variable.
Please re-open if there is still a problem.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Great, thank you!!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.