Add documentation files and GitHub Actions workflow for deploying docs#12
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive documentation suite for the PHP GUI library using Docsify, covering installation, architecture, and usage examples. While the documentation is well-structured, several issues need to be addressed: there are broken links in the sidebar to missing widget documentation files, a missing architectural diagram (system.svg), and bugs in the provided code examples, such as the use of undefined variables and non-graceful application exits. Additionally, there are inconsistencies in the widget listings and a missing CSS file for the code-copying plugin.
| $button = new Button($parent, [ | ||
| 'text' => 'Click', | ||
| 'command' => function() { | ||
| echo "Clicked!\n"; | ||
| } | ||
| ]); | ||
|
|
||
| // Input enter key | ||
| $input = new Input($parent, ['text' => 'Type here...']); | ||
| $input->onEnter(function() use ($input) { | ||
| echo "Entered: " . $input->getValue() . "\n"; | ||
| }); |
There was a problem hiding this comment.
The code example uses an undefined variable $parent when creating the Button and Input widgets. This will cause a fatal error for anyone trying to run the example. Please define $parent or replace it with a valid parent widget ID, like $window->getId() from the preceding example.
| $button = new Button($parent, [ | |
| 'text' => 'Click', | |
| 'command' => function() { | |
| echo "Clicked!\n"; | |
| } | |
| ]); | |
| // Input enter key | |
| $input = new Input($parent, ['text' => 'Type here...']); | |
| $input->onEnter(function() use ($input) { | |
| echo "Entered: " . $input->getValue() . "\n"; | |
| }); | |
| $button = new Button($window->getId(), [ | |
| 'text' => 'Click', | |
| 'command' => function() { | |
| echo "Clicked!\n"; | |
| } | |
| ]); | |
| // Input enter key | |
| $input = new Input($window->getId(), ['text' => 'Type here...']); | |
| $input->onEnter(function() use ($input) { | |
| echo "Entered: " . $input->getValue() . "\n"; | |
| }); |
| - [Installation](getting-started.md) | ||
| - [Architecture](architecture.md) | ||
|
|
||
| - **Widgets** |
There was a problem hiding this comment.
|
|
||
| PHP GUI uses PHP's FFI (Foreign Function Interface) extension to bridge PHP code with native Tcl/Tk libraries. This enables building desktop GUI applications in pure PHP without any compiled extensions. | ||
|
|
||
| <img src="system.svg" style="width:250px" alt="System Architecture Diagram"> |
| - [Button](Button.md) | ||
| - [Label](Label.md) | ||
| - [Input](Input.md) | ||
| - [Entry](Entry.md) |
| The event system uses a temp-file bridge pattern: | ||
|
|
||
| 1. A Tcl event fires (e.g., button click) | ||
| 2. The callback ID is written to `/tmp/phpgui_callback.txt` |
There was a problem hiding this comment.
The documentation states that the callback ID is written to /tmp/phpgui_callback.txt. However, the implementation uses sys_get_temp_dir() to determine the temporary directory, which is more portable. The documentation should be updated to reflect this for accuracy.
| 2. The callback ID is written to `/tmp/phpgui_callback.txt` | |
| 2. The callback ID is written to `phpgui_callback.txt` in the system's temporary directory. |
| $fileMenu->addCommand('Exit', function () { | ||
| exit(); | ||
| }, ['foreground' => 'red']); |
There was a problem hiding this comment.
Using exit() directly can lead to an abrupt termination of the application, potentially skipping cleanup procedures defined in destructors. It's better to use the application's quit() method for a more graceful shutdown. The $app variable also needs to be passed into the closure's scope.
| $fileMenu->addCommand('Exit', function () { | |
| exit(); | |
| }, ['foreground' => 'red']); | |
| $fileMenu->addCommand('Exit', function () use ($app) { | |
| $app->quit(); | |
| }, ['foreground' => 'red']); |
| - **Modern PHP** — Requires PHP 8.1+ with FFI extension | ||
| - **Simple API** — Clean widget classes that abstract Tcl/Tk complexity | ||
| - **Event-driven** — PHP callbacks for interactive widgets | ||
| - **14 Widgets** — Window, Button, Label, Input, Entry, Frame, Canvas, Menu, Menubutton, Checkbutton, Combobox, Message, TopLevel, and Image |
There was a problem hiding this comment.
This list includes Entry as a widget, but the library provides an Input widget that uses the Tcl entry widget. There is no separate Entry class. This is confusing for users. I recommend removing Entry from the list and adjusting the widget count for clarity. Also, Image is listed here but not in the sidebar, which is inconsistent.
| - **14 Widgets** — Window, Button, Label, Input, Entry, Frame, Canvas, Menu, Menubutton, Checkbutton, Combobox, Message, TopLevel, and Image | |
| - **13 Widgets** — Window, Button, Label, Input, Frame, Canvas, Menu, Menubutton, Checkbutton, Combobox, Message, TopLevel, and Image |
| <script src="https://cdn.jsdelivr.net/npm/docsify@4/lib/docsify.min.js"></script> | ||
| <!-- Plugins --> | ||
| <script src="https://cdn.jsdelivr.net/npm/docsify@4/lib/plugins/search.min.js"></script> | ||
| <script src="https://cdn.jsdelivr.net/npm/docsify-copy-code@2"></script> |
There was a problem hiding this comment.
The docsify-copy-code plugin requires a CSS file for styling, which is missing. This can lead to incorrect styling for the "Copy" button. Please add the following line to the <head> section of this file to ensure the copy button is styled correctly:
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/docsify-copy-code/dist/style.css">
No description provided.