Skip to content

Add documentation files and GitHub Actions workflow for deploying docs#12

Merged
developersharif merged 1 commit into
mainfrom
docv1.5
Mar 30, 2026
Merged

Add documentation files and GitHub Actions workflow for deploying docs#12
developersharif merged 1 commit into
mainfrom
docv1.5

Conversation

@developersharif

Copy link
Copy Markdown
Owner

No description provided.

@developersharif
developersharif merged commit 74e1d4e into main Mar 30, 2026
3 checks passed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread docs/getting-started.md
Comment on lines +134 to +145
$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";
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

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.

Suggested change
$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";
});

Comment thread docs/_sidebar.md
- [Installation](getting-started.md)
- [Architecture](architecture.md)

- **Widgets**

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

This section and the following ones list many widget documentation pages (e.g., Window.md, Button.md) that are not included in this pull request. This will result in broken links on the documentation site. Please either add these missing files or comment out the links until the pages are ready.

Comment thread docs/architecture.md

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">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The <img> tag references system.svg, but this file is not included in the pull request. This will result in a broken image in the documentation. Please add the SVG file to the repository.

Comment thread docs/_sidebar.md
- [Button](Button.md)
- [Label](Label.md)
- [Input](Input.md)
- [Entry](Entry.md)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The Input widget is already listed, and there is no separate Entry widget class in the project. Listing Entry here is redundant and confusing for users. Please remove this line.

Comment thread docs/architecture.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`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
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.

Comment thread docs/example.md
Comment on lines +102 to +104
$fileMenu->addCommand('Exit', function () {
exit();
}, ['foreground' => 'red']);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
$fileMenu->addCommand('Exit', function () {
exit();
}, ['foreground' => 'red']);
$fileMenu->addCommand('Exit', function () use ($app) {
$app->quit();
}, ['foreground' => 'red']);

Comment thread docs/home.md
- **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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
- **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

Comment thread docs/index.html
<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>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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">

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.

1 participant