add xtailwind and xtailwind2 themes#4
Conversation
|
Caution Review failedPull request was closed or merged during review WalkthroughTwo complete XOOPS 2.5.12 themes ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.46)Invalid entry in excludePaths: If the excluded path can sometimes exist, append (?) parameters: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4 +/- ##
=========================================
Coverage 19.22% 19.23%
Complexity 7584 7584
=========================================
Files 621 621
Lines 40085 40085
=========================================
+ Hits 7707 7709 +2
+ Misses 32378 32376 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces two new Tailwind CSS + DaisyUI-based themes (xtailwind and xtailwind2) to the XOOPS theme set, including compiled CSS assets, template structure, theme bootstrapping (theme_autorun.php), and accompanying documentation/build configs.
Changes:
- Add the
xtailwind2theme (curated palettes, new shell/templates, Tailwind+DaisyUI config, compiled CSS, docs). - Add the
xtailwindtheme (reference/proof-of-concept Tailwind+DaisyUI theme + tutorial/docs/config). - Add a PowerShell helper script intended to generate/write
xtailwind2theme files.
Reviewed changes
Copilot reviewed 48 out of 50 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/write_xtailwind2.ps1 | Adds a local script to generate/write out the xtailwind2 theme files. |
| htdocs/themes/xtailwind2/TUTORIAL.md | Theme-specific guide explaining structure, palette system, and customization. |
| htdocs/themes/xtailwind2/tpl/rightBlock.tpl | Right sidebar rail template for blocks. |
| htdocs/themes/xtailwind2/tpl/nav-menu.tpl | Main navigation + search + theme picker + mobile drawer markup. |
| htdocs/themes/xtailwind2/tpl/leftBlock.tpl | Left sidebar rail template for blocks. |
| htdocs/themes/xtailwind2/tpl/index.php | Directory guard (404) for tpl/. |
| htdocs/themes/xtailwind2/tpl/inboxAlert.tpl | PM toast alert template (Alpine-style directives). |
| htdocs/themes/xtailwind2/tpl/content-zone.tpl | Main content wrapper + page-top block regions. |
| htdocs/themes/xtailwind2/tpl/blockContent.tpl | Shared block wrapper + admin “edit block” link. |
| htdocs/themes/xtailwind2/theme.tpl | Main page shell, homepage hero, and theme-switching JS. |
| htdocs/themes/xtailwind2/theme.ini | XOOPS theme metadata for xtailwind2. |
| htdocs/themes/xtailwind2/theme_autorun.php | Theme bootstrap: renderer selection + Smarty vars for theme palettes. |
| htdocs/themes/xtailwind2/tailwind.config.js | Tailwind scan paths, theme extensions, and DaisyUI palette definitions. |
| htdocs/themes/xtailwind2/README.md | Theme overview, behavior notes, and build instructions. |
| htdocs/themes/xtailwind2/package.json | Theme-local build scripts/deps for Tailwind compilation. |
| htdocs/themes/xtailwind2/LICENSE | Bundled license text (GPL v2). |
| htdocs/themes/xtailwind2/language/index.php | Directory guard (404) for language/. |
| htdocs/themes/xtailwind2/language/english/main.php | English language constants for theme strings. |
| htdocs/themes/xtailwind2/language/english/index.php | Directory guard (404) for language/english/. |
| htdocs/themes/xtailwind2/index.php | Directory guard (404) for theme root. |
| htdocs/themes/xtailwind2/images/index.php | Directory guard (404) for images/. |
| htdocs/themes/xtailwind2/css/styles.css | Compiled/minified Tailwind+DaisyUI CSS output. |
| htdocs/themes/xtailwind2/css/input.css | Tailwind source CSS (custom layers/components/utilities). |
| htdocs/themes/xtailwind2/css/index.php | Directory guard (404) for css/. |
| htdocs/themes/xtailwind/TUTORIAL.md | General tutorial for building Tailwind themes for XOOPS. |
| htdocs/themes/xtailwind/tpl/rightBlock.tpl | Right sidebar block template for xtailwind. |
| htdocs/themes/xtailwind/tpl/nav-menu.tpl | Navigation + theme switcher + Alpine-driven mobile nav (xtailwind). |
| htdocs/themes/xtailwind/tpl/leftBlock.tpl | Left sidebar block template for xtailwind. |
| htdocs/themes/xtailwind/tpl/index.php | Directory guard (404) for tpl/. |
| htdocs/themes/xtailwind/tpl/inboxAlert.tpl | PM toast alert template (Alpine-style directives). |
| htdocs/themes/xtailwind/tpl/content-zone.tpl | Main content wrapper + page-top blocks (xtailwind). |
| htdocs/themes/xtailwind/tpl/blockContent.tpl | Shared block wrapper + admin edit shortcut (xtailwind). |
| htdocs/themes/xtailwind/theme.tpl | Main page shell + theme restoration + Alpine include (xtailwind). |
| htdocs/themes/xtailwind/theme.ini | XOOPS theme metadata for xtailwind. |
| htdocs/themes/xtailwind/theme_autorun.php | Theme bootstrap: renderer selection + Smarty theme list. |
| htdocs/themes/xtailwind/tailwind.config.js | Tailwind config including safelist and DaisyUI theme list. |
| htdocs/themes/xtailwind/README.md | Theme overview, install notes, and rebuild instructions. |
| htdocs/themes/xtailwind/package.json | Theme-local build scripts/deps for Tailwind compilation. |
| htdocs/themes/xtailwind/LICENSE | Bundled license text (GPL v2). |
| htdocs/themes/xtailwind/language/index.php | Directory guard (404) for language/. |
| htdocs/themes/xtailwind/language/english/main.php | English language constants for xtailwind strings. |
| htdocs/themes/xtailwind/language/english/index.php | Directory guard (404) for language/english/. |
| htdocs/themes/xtailwind/index.php | Directory guard (404) for theme root. |
| htdocs/themes/xtailwind/images/index.php | Directory guard (404) for images/. |
| htdocs/themes/xtailwind/css/input.css | Tailwind source CSS (custom layer + block normalization). |
| htdocs/themes/xtailwind/css/index.php | Directory guard (404) for css/. |
| htdocs/themes/xtailwind/.gitignore | Ignores node build artifacts while keeping compiled CSS committed. |
Files not reviewed (2)
- htdocs/themes/xtailwind/package-lock.json: Language not supported
- htdocs/themes/xtailwind2/package-lock.json: Language not supported
| <script> | ||
| (function() { | ||
| const stored = localStorage.getItem('xtailwind2-theme'); | ||
| const prefersDark = window.matchMedia('(prefers-color-scheme: dark)').matches; | ||
| const theme = stored || (prefersDark ? 'noctis' : 'atelier'); | ||
| document.documentElement.setAttribute('data-theme', theme); | ||
| })(); | ||
| </script> | ||
|
|
||
| <script src="<{$xoops_url}>/browse.php?Frameworks/jquery/jquery.js"></script> | ||
| <link rel="alternate" type="application/rss+xml" title="" href="<{xoAppUrl 'backend.php'}>"> |
There was a problem hiding this comment.
tpl/inboxAlert.tpl uses Alpine.js directives (x-data, x-show, x-transition, @click), but this theme template never loads Alpine.js (only jQuery). Without Alpine, the PM toast will never auto-hide and the close button won’t work. Include the shared Alpine.js bundle (as done in xtailwind/theme.tpl) or remove the Alpine directives and implement equivalent behavior with plain JS.
| <a class="btn btn-ghost rounded-full px-4 text-sm font-medium" href="<{if $cat.category_url neq ''}><{$cat.category_url|escape}><{else}>#<{/if}>" target="<{$cat.category_target}>"> | ||
| <{$cat.category_title|escape}> | ||
| </a> |
There was a problem hiding this comment.
Top-level category links can set target="_blank" via $cat.category_target, but unlike submenu links they don’t add rel="noopener noreferrer". Add the same conditional rel when opening a new tab to prevent reverse-tabnabbing.
| <a class="block rounded-2xl px-3 py-2 text-sm text-base-content/80 hover:bg-base-200/70" href="<{if $subItem.url neq ''}><{$subItem.url|escape}><{else}>#<{/if}>" target="<{$subItem.target}>" <{if $subItem.target == '_blank'}> rel="noopener noreferrer"<{/if}>><{$subItem.title|escape}></a> | ||
| </li> | ||
| <{/foreach}> | ||
| </ul> | ||
| </details> | ||
| <{else}> | ||
| <a class="block rounded-3xl border border-base-300/70 bg-base-100/65 px-4 py-3 font-semibold" href="<{if $cat.category_url neq ''}><{$cat.category_url|escape}><{else}>#<{/if}>" target="<{$cat.category_target}>"><{$cat.category_title|escape}></a> | ||
| <{/if}> |
There was a problem hiding this comment.
Same issue as desktop: mobile category links use target="<{$cat.category_target}>" without adding rel="noopener noreferrer" when the target is _blank. Add the conditional rel attribute for security consistency.
| <button id="xtailwind2-mobile-toggle" class="btn btn-ghost rounded-full xl:hidden" type="button" aria-expanded="false" aria-controls="xtailwind2-mobile-panel"> | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="22" height="22" class="h-5 w-5" fill="none" viewBox="0 0 24 24" stroke="currentColor"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M4 6h16M4 12h16M4 18h16"/></svg> | ||
| </button> |
There was a problem hiding this comment.
The mobile menu toggle button is icon-only and currently has no accessible name. Add an aria-label (e.g., "Open menu"/"Close menu") so screen readers can announce the control properly.
| <form role="search" action="<{xoAppUrl 'search.php'}>" method="get" class="flex gap-2"> | ||
| <input type="text" name="query" class="input input-bordered w-full rounded-full" placeholder="<{$smarty.const.THEME_SEARCH_TEXT|default:'Search'}>"> | ||
| <input type="hidden" name="action" value="results"> | ||
| <button class="btn btn-primary rounded-full" type="submit">Go</button> | ||
| </form> | ||
| <{/if}> |
There was a problem hiding this comment.
The mobile search submit button text is hardcoded as "Go" even though a translatable constant (THEME_SEARCH_BUTTON) exists. Use the language constant (with a sensible default) to keep the theme fully localizable.
| $base = 'C:\wamp64\www\2512menus\themes\xtailwind2' | ||
|
|
There was a problem hiding this comment.
This script hardcodes a local absolute path (C:\wamp64\www\...) as the output base directory, which makes it unusable for other developers/CI and easy to accidentally overwrite local files. Consider taking the destination path as a parameter (or deriving it relative to the repo) and validating it before writing.
| '@ | Set-Content -Path (Join-Path $base 'package.json') -Encoding UTF8 | ||
|
|
There was a problem hiding this comment.
Set-Content -Encoding UTF8 can emit a UTF-8 BOM in Windows PowerShell, which is problematic for generated PHP files (it can trigger "headers already sent"). Prefer writing PHP/INI/TPL files as UTF-8 without BOM (e.g., utf8NoBOM where available) to avoid subtle runtime issues.
| { | ||
| "name": "xtailwind2", | ||
| "version": "0.2.0", | ||
| "description": "Art-directed Tailwind CSS + DaisyUI theme for XOOPS 2.5", | ||
| "private": true, |
There was a problem hiding this comment.
xtailwind2 includes package.json but there is no per-theme .gitignore like xtailwind/.gitignore to keep node_modules/ and npm logs out of commits. Add a .gitignore in this theme directory (or a repo-level one) to avoid accidental vendor check-ins when running npm install.
| - The root `<html>` starts with `data-theme="atelier"`, then swaps to the remembered theme before paint. | ||
| - The homepage gets a dedicated hero section when `$xoops_page == "index"`. | ||
| - Footer blocks render as lighter support panels instead of repeating the full card treatment from the sidebars. | ||
| - The theme prefers `XoopsFormRendererTailwind` (XOOPS 2.5.13+) so forms stay aligned with the rest of the Tailwind shell. On older XOOPS versions without that renderer, it gracefully falls back to `XoopsFormRendererBootstrap5` — forms still render correctly because DaisyUI aliases most Bootstrap 5 form component classes. |
There was a problem hiding this comment.
This README states that the Bootstrap 5 fallback renderer stays styled because "DaisyUI aliases most Bootstrap 5 form component classes", but the compiled CSS shipped with the theme does not include Bootstrap selectors like .form-control / .form-select. Please update the documentation or add the necessary alias CSS so the fallback behavior matches what’s described.
| - The theme prefers `XoopsFormRendererTailwind` (XOOPS 2.5.13+) so forms stay aligned with the rest of the Tailwind shell. On older XOOPS versions without that renderer, it gracefully falls back to `XoopsFormRendererBootstrap5` — forms still render correctly because DaisyUI aliases most Bootstrap 5 form component classes. | |
| - The theme prefers `XoopsFormRendererTailwind` (XOOPS 2.5.13+) so forms stay aligned with the rest of the Tailwind shell. On older XOOPS versions without that renderer, it falls back to `XoopsFormRendererBootstrap5` for functional compatibility. The shipped theme CSS does not alias Bootstrap 5 form classes such as `.form-control` or `.form-select`, so Bootstrap-rendered forms may not match the full Tailwind/DaisyUI presentation until the Tailwind renderer is available or additional compatibility CSS is added. |
| ### End users (site administrators) | ||
| - XOOPS 2.5.11+ (RTL auto-detection requires 2.5.12+) | ||
| - PHP 8.2+ | ||
| - `XoopsFormRendererTailwind` class in XOOPS core is **recommended** (shipped with XOOPS 2.5.13+). If your XOOPS version doesn't include it, the theme automatically falls back to the Bootstrap 5 form renderer — forms will still work and look mostly correct because DaisyUI aliases most Bootstrap 5 form classes. For pixel-perfect form styling, add `XoopsFormRendererTailwind.php` to `htdocs/class/xoopsform/renderer/` manually. |
There was a problem hiding this comment.
This README claims the Bootstrap 5 fallback renderer looks mostly correct because DaisyUI aliases Bootstrap form classes, but the compiled CSS does not include selectors like .form-control. Update this statement or add explicit styling/aliases for the Bootstrap-rendered controls so the fallback matches the documented behavior.
| - `XoopsFormRendererTailwind` class in XOOPS core is **recommended** (shipped with XOOPS 2.5.13+). If your XOOPS version doesn't include it, the theme automatically falls back to the Bootstrap 5 form renderer — forms will still work and look mostly correct because DaisyUI aliases most Bootstrap 5 form classes. For pixel-perfect form styling, add `XoopsFormRendererTailwind.php` to `htdocs/class/xoopsform/renderer/` manually. | |
| - `XoopsFormRendererTailwind` class in XOOPS core is **recommended** (shipped with XOOPS 2.5.13+). If your XOOPS version doesn't include it, the theme automatically falls back to the Bootstrap 5 form renderer — forms will still function, but Bootstrap-specific control classes are not styled by the compiled xTailwind CSS in the same way as the Tailwind renderer output. For consistent, theme-matched form styling, add `XoopsFormRendererTailwind.php` to `htdocs/class/xoopsform/renderer/` manually. |
Summary by CodeRabbit
New Features
Documentation