Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pdfGalery component #84

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

pdfGalery component #84

wants to merge 11 commits into from

Conversation

paproc
Copy link
Contributor

@paproc paproc commented Oct 16, 2024

No description provided.

Comment on lines +41 to 52
try {
$info = static::getPdfProp($file->getRealPath());
$name = $info["Title"] ?? ($info["title"] ?? '');
} catch (\Exception $e) {
$name = '';
}

$wwwPath = substr($file->getPathname(), strlen($wwwDir));
$pdfs[] = [
'src' => $wwwPath,
'name' => $file->getBasename('.pdf'),
'name' => $name ?: $file->getBasename('.pdf'),
];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buď zrovna defaultni na název souboru (asi preferované), nebo to nastav na null a použij null coalesce, takto to vypadá, jak kdyby to PDF nemělo být jméno, pokud nemá nastavený title.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tohle navíc chytá když budeš mít nastavený title na ''.

{
$style ??= $path == null ? "Buttons" : "List";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tohle bych vyřešil po vzoru ImageGallery různými render metodami, než to vkládat jako styl.

{
$style ??= $path == null ? "Buttons" : "List";
$renderFile = __DIR__ . DIRECTORY_SEPARATOR . 'pdfGallery' . $style . '.latte';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rozhodit do dvou renderů, viz předchozí point

$style ??= $path == null ? "Buttons" : "List";
$renderFile = __DIR__ . DIRECTORY_SEPARATOR . 'pdfGallery' . $style . '.latte';

$path ??= '/media/download' . strtolower(str_replace(':', '/', $this->getPresenter()->getAction(true)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nepřijde mi dobré to takto magicky defaultovat, taky proto, že a) kvůli modulům a b) kvůli přehlednosti, kde ta komponenta bere věci. Taky např. přesuneš stránku a najednou se ti rozbijou PDFka a nebudeš vědět proč.

Zároveň se snažím držet konvenci, že pokud jsou nějaké soubory zmíněny v kódu, pak mají být v gitu, pokud jsou tahány dynamicky, pak mají být v media. Tedy např. ty dokumenty pro učitele bych dal do www a odkázal se na ně cestou.

Comment on lines +82 to +96
if ($this->notRenderToStr == true) {
$this->template->render($renderFile);
return;
}
$this->notRenderToStr = $this->template->renderToString($renderFile);
}

public function renderToString(?string $path = null, string $style = "default"): string
{
$this->notRenderToStr = false;
$this->render($path, $style);
if (is_bool($this->notRenderToStr)) {
throw new \Exception('Component ' . __CLASS__ . ' did not render properly!');
}
return $this->notRenderToStr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

K čemu je toto?

Comment on lines +103 to +106
public function hasFiles(?string $path = null): bool
{
$path ??= $this->wwwDir . strtolower(str_replace(':', '/', $this->getPresenter()->getAction(true)));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zbytečně duplikuješ magic pro získání cesty, klidně to tady může být jako povinný argument.
Edit: jo, už chápu, aby se to dalo volat z presenterů. Každopádně stejně jako výše bych to nechal povinné.

Comment on lines +112 to 164

/**
* magic from http://www.fpdf.org/en/script/script59.php
* @param string $file filename
* @return array whose keys are the names of the properties found in the file
*/
public static function getPdfProp($file): array
{
$f = fopen($file, 'rb');
if (!$f) {
return [];
}
//Read the last 16KB
fseek($f, -16384, SEEK_END);
$s = fread($f, 16384);

//Extract cross-reference table and trailer
if (!preg_match("/xref[\r\n]+(.*)trailer(.*)startxref/s", $s, $a)) {
return [];
}
$xref = $a[1];
$trailer = $a[2];

//Extract Info object number
if (!preg_match('/Info ([0-9]+) /', $trailer, $a)) {
return [];
}
$object_no = (int)$a[1];

//Extract Info object offset
$lines = preg_split("/[\r\n]+/", $xref);
$line = $lines[1 + $object_no];
$offset = (int) $line;
if ($offset == 0) {
return [];
}
//Read Info object
fseek($f, $offset, SEEK_SET);
$s = fread($f, 1024);
fclose($f);

//Extract properties
if (!preg_match('/<<(.*)>>/Us', $s, $a)) {
return [];
}
$n = preg_match_all('|/([a-z]+) ?\((.*)\)|Ui', $a[1], $a);
$prop = array();
for ($i = 0; $i < $n; $i++) {
$prop[$a[1][$i]] = $a[2][$i];
}
return $prop;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toto je vyloženě magic. Jestli chceš tahat info ze souborů, tak na to použij nějakou knihovnu, nebo to chce vymyslet jinak, za mě je třeba ok používat názvy souborů z filesystemu, mezery by neměly být v názvech problém a umožní ti to ty soubory jednodušše přejmenovat, než když to bude tahat jako info ze souboru.

@@ -0,0 +1,10 @@
{varType array[] $pdfs}
<div>
{foreach $pdfs as $pdffile}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dobré dodržovat camelCase, tedy $pdfFile

<h2 class='inner-container' style='color: red'>TODO pdf verze souborů: Pravidla, Návratka</h2> {* TODO *}
<section class="inner-container">
<h2>Pravidla, Návratka</h2>{* TODO nahrát soubory www/vyfuk/media/download/default/section/teachers *}
{control pdfGallery}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tady bych se klidně cestou odkázal na zagitované soubory ve www, protože předpokládáš, že tu budou a není to něco typu dynamicky tahané obrázky nebo soubory k akcím.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants