Skip to content

fix(cssholmes): integrate presentation and review corrections - #10

Merged
mambax7 merged 8 commits into
masterfrom
review/alpha1-runtime-ui-followup
Oct 27, 2026
Merged

mambax7 merged 8 commits into
masterfrom
review/alpha1-runtime-ui-followup

Conversation

@mambax7

@mambax7 mambax7 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Bring the presentation changes from merged #7 into master and address the valid runtime and UI findings from reviews of #6 and #7. #7 was merged into the runtime branch, so its changes are not yet in master. This follow-up contains 89 files, below the review limit.

Changes

  • Require Export permission for designer downloads and both style-set export endpoints.
  • Keep token preview, apply and rollback within the theme directory, including when theme.json is a symlink.
  • Bound import payloads and history size without silently discarding rollback records.
  • Correct named colors, both image dimensions, associated form labels, missing-main counters, dark-mode selectors and widget-root diagnostics.
  • Avoid repeated mainfile initialization, uninstall backup collisions and duplicate GD status entries.
  • Escape inspected and exported overlay text, derive asset URLs from the module script, and validate font lists without an ambiguous repeating regex. All nine subsequent CodeQL alerts are cleared.

Validation

The complete candidate, including the test branch in #9, passed PHP 8.4/8.5 isolated suites (35 PHPUnit tests, 115 assertions; 50 endpoint checks), browser regressions and scoped PHPStan. A disposable installed XOOPS 2.7 site passed administrator login, xStyles designer controls, save/reload, visitor styles, permissions, CSRF and restore. Windows cannot create the test symlink; Linux CI in #9 exercises that case.

Merge order

Merge this PR into master first. The documentation follow-up and #9 depend on it; retarget each to master after its predecessor is merged. Do not merge a later step into its temporary stack base.

Copilot AI balanced review requested due to automatic review settings October 27, 2026 02:40
@sourcery-ai

sourcery-ai Bot commented Oct 27, 2026

Copy link
Copy Markdown

Sorry @mambax7, your pull request is larger than the review limit of 150,000 diff characters

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Walkthrough

The pull request adds browser diagnostics, theme editing, gallery previews, and theme-set management. It also updates export permissions, analyzer checks, workbench limits, module documentation, and directory fallback responses.

Changes: Diagnostics and theme workbench

Layer / File(s) Summary
Diagnostic profiles and examples
assets/css/holmes.css, assets/css/profiles/*, templates/cssholmes_testsuite.tpl, templates/cssholmes_xtf_testsuite.tpl
Adds profile-based CSS markers and hover messages for HTML, accessibility, layout, and XTF/XMF findings. Adds test-suite examples for these profiles.
Holmes overlay runtime
assets/js/holmes.js
Adds profile selection, finding counts, element inspection, editing, export, and diagnostic overlays controlled by page configuration and permissions.
Theme designer and diagnostics
assets/js/designer.js, assets/js/designer-diagnostics.js, assets/css/designer.css, language/english/main.php, language/english/diagnostics.php
Adds draft rule editing, validation, preview, undo and redo, persistence, export, and token and contrast diagnostics.
Gallery draft previews
assets/js/gallery-designer.js, assets/css/gallery-designer.css
Adds published-versus-draft and responsive previews using validated gallery frames.
Theme-set administration and export
assets/js/theme-sets-admin.js, assets/css/theme-sets-admin.css, class/Designer/Access.php, preloads/designer.php, designer.php, sets.php, language/english/admin.php
Adds theme-set and palette administration, import review, and export permission checks in the capability data and endpoints.
Analyzer and workbench validation
class/Analyzer/*, class/Workbench/*, class/Common/ServerStats.php
Tightens color and dimension checks, limits import and history sizes and counts, and resolves the theme manifest through the theme-root path resolver.
Module documentation and setup
language/*, templates/cssholmes_index.tpl, include/oninstall.php, include/onuninstall.php, assets/css/admin.css, assets/css/menu.css, assets/css/style.css
Updates help, translations, and module metadata; changes install loading and the upload-directory backup suffix.
Directory access fallbacks
assets/**/index.html, assets/**/index.php, language/**/index.html, language/**/index.php, templates/**/index.html, templates/**/index.php, index.html
Removes history-back index pages and sets PHP fallback responses to 404 or 403.

Priority: Pending latest changes

Estimated code review effort: 5 (Critical) | ~120 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  actor Editor
  participant CssHolmesDesigner
  participant ThemeEndpoint
  participant PageStyles
  Editor->>CssHolmesDesigner: Edit theme rule
  CssHolmesDesigner->>ThemeEndpoint: Request theme state or save changes
  ThemeEndpoint-->>CssHolmesDesigner: Return theme state and permissions
  CssHolmesDesigner->>PageStyles: Apply draft preview
Loading

Possibly related PRs

Merge Risk: High · up to 45692

Do not merge yet: viewing page content in the overlay can execute injected markup, and users without Export permission can retrieve stored set documents through preview. The remaining editor, accessibility, and diagnostic issues also need correction or explicit acceptance.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

PHPStan (2.3.0)

PHPStan reported 1 non-file analysis error(s); analysis is incomplete

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread assets/js/designer.js Fixed
Comment thread assets/js/holmes.js Fixed
Comment thread assets/js/holmes.js Fixed
Comment thread assets/js/holmes.js Fixed
Comment thread assets/js/holmes.js Fixed
Comment thread assets/js/holmes.js Fixed
Comment thread assets/js/holmes.js Fixed
Comment thread assets/js/holmes.js Fixed
Comment thread assets/js/holmes.js Fixed
@mambax7
mambax7 merged commit ef4c676 into master Oct 27, 2026
4 checks passed
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.

3 participants