search

Geeklog-Core / geeklog

Geeklog - The Secure CMS.
https://www.geeklog.net
24 stars 19 forks source link

Printable Pages should use COM_createHTMLDocument to reduce Duplicate Code #1051

Closed eSilverStrike closed 2 years ago

eSilverStrike commented 4 years ago

For Geeklog v2.2.1sr1 I had to update both The printable page code for articles and staticpages as it was missing some css and javascript which makes the pages presentable for printing (especially if pages have HTML in them and use autotags). I basically copied the code from COM_createHTMLDocument to SP_printPage in the functions.inc for staticpages and in article.php

This creates duplicate code and will create problems in the future if COM_createHTMLDocument changes and the code for these printable pages do not.

Also these printable pages have the doctype etc hard coded in their template files which they should not.

What we should do is update COM_createHTMLDocument (or create separate functions that they can share???) to allow for other page types like print pages (or just simple pages??). These pages should be able to set their own template(s) file and then COM_createHTMLDocument (or a shared function) could then set all the doc type, language variables, css, javascript needed while skipping adding structured data, left and right blocks, etc...

Plus if you have a print page type for COM_createHTMLDocument then it could set automatically extra things that are common to all print pages like NOINDEX

<meta name="robots" content="NOINDEX"{xhtml}>

as well as the print.css file available from some themes. (see code in article.php)

eSilverStrike commented 2 years ago

@mystralkk I created a new function called COM_createHTMLPrintableDocument. It is based on COM_createHTMLDocument. Even though COM_createHTMLPrintableDocument is fairly similar to COM_createHTMLDocument I thought it was best ??? to create the new function than implement the printable page function and changes into COM_createHTMLDocument.

I have also updated the forum to use COM_createHTMLPrintableDocument. Do any other plugins offer printable pages?

mystralkk commented 2 years ago

@eSilverStrike to be honest, I am not sure if it is best to create a new function like COM_createHTMLPrintableDocument, but since you have worked a lot to fix this issue, I think it is better to accept the changes you made.

I have also updated the forum to use COM_createHTMLPrintableDocument. Do any other plugins offer printable pages?

As far as I know, there is no such plugin.

eSilverStrike commented 2 years ago

You mean instead of combining it with COM_createHTMLDocument?

mystralkk commented 2 years ago

I mean that in theory, it is better to have one function or one class to do two similar jobs instead of having two functions, but that creating one such function is rather hard in practice in this case. In fact, I tried creating such a class that creates a document for/not for printing, but couldn't implement the class neatly.

eSilverStrike commented 2 years ago

Completely agree.