Repository navigation
Unify administration and plugin interfaces and fix mobile form layouts - #471
Conversation
|
Important Review skippedToo many files! This PR contains 175 files, which is 75 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
📒 Files selected for processing (175)
You can disable this status message by setting the
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The settings tablist this PR removes is still clicked by 23 Playwright locators in four specs outside the diff, so those suites will fail. Restricting content editing to the site language also makes per-visitor-language texts uneditable. Please confirm that is intended.
Reviewed changes
Reviewed the full diff (single commit 8a4aec8) across core views, plugin views, CSS/JS assets, I18n, DesiderataPlugin and tests.
- Shared staff header:
partials/workspace-header.phpreplaces each view's own header markup through captured slots. All ~110dirname(__DIR__, N)include paths resolve, and every changed PHP file passesphp -l. - Admin shell: theme tokens are now inlined from
ThemeColorizer. Adds a nav search, a grouped Settings submenu, a focus trap and Escape for the mobile menu, andaria-currenton the active nav link. Quick Actions move to the bottom of the nav. - Settings page: the
[data-settings-tab]tablist becomes real?tab=links, switched client-side withpushState/popstate. interface.js: plugin directory filtering, nav highlight, keyboard-reachable table scroll regions, and form-action visibility.- Content language: the new
I18n::getContentLocales()limits the catalogue/events headers, the Desiderata CMS section and the Dewey default to the installation locale.mergeCmsTextskeeps translations that were not submitted, and the new unit test passes locally. - Dewey editor: the tabs become a
<select>, with a stale-response guard and Save disabled while loading. - Book Club public CSS moves from repeated inline
<style>blocks tobook-club-ui.css.
Run locally (all pass): desiderata-cms-language, frontend-layout-variants, frontend-partials, frontend-theme-a11y, theme-readable-accent, additional-css-no-style-tag and ci-check-locales.py.
⚠️ E2E specs still click the removed settings tabs
settings/index.php no longer renders [data-settings-tab], but 23 locators in four specs outside the diff still call .click() on it. Each one will hit the Playwright timeout. The PR updated catalog-header-cms.spec.js and desiderata-extended.spec.js, but not these.
Technical details
# Stale `[data-settings-tab]` locators
## Affected sites
- tests/label-pdf-content.spec.js:146
- tests/multiple-copy-loans-238.spec.js:270, 288
- tests/full-test.spec.js:1884, 1891, 1898, 1905, 1912, 1919, 1926, 1933, 1957, 3131
- tests/admin-features.spec.js:102, 118, 133, 157, 177, 199, 226, 241, 257, 268
## Required outcome
- Specs reach each settings section using the new markup.
## Suggested approach
- Target the in-page switcher link, e.g. open `#settings-page-menu summary` and click `#settings-page-menu [data-settings-section="labels"]`, or simply `page.goto('/admin/settings?tab=labels')`.
- Note that `[data-settings-section="x"]` alone matches two elements (sidebar link + page menu link), so scope it to avoid a strict-mode violation.
- `grep -rn "data-settings-tab" tests/` should return nothing afterwards.claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No critical issues — both prior concerns are addressed. One test synchronisation gap is noted inline, and one E2E assertion outside the diff still targets the old nav class.
Reviewed changes
Reviewed c95ba7a against the prior review at 8a4aec8.
- Translation editing restored:
I18n::getContentLocales()now takes an optional requested locale and falls back to the site locale for inactive or non-string values. A newcontent-language-switcher.phppartial adds a collapsed "Altre traduzioni" menu to the catalogue/events header forms and the Desiderata card. Both save handlers redirect back to the selected translation. - Settings hash handling: a URL hash is only treated as a section when it names a known section, so
#catalog-header-formsurvives as a content anchor. - Homepage section rows: drag now starts only from a dedicated 44px handle, which also reorders with ArrowUp/ArrowDown. The Visible toggle moves to its own row below 400px, and the order badge text is now translated.
- E2E locators: all 23
[data-settings-tab]clicks now go through the newopenSettingsSectionhelper. The catalogue-header and Desiderata specs also round-trip a secondary translation.
Checked locally: php -l passes on every PHP file the commit touched, desiderata-cms-language.unit.php passes 5/5 including the new requested-locale cases, and ci-check-locales.py passes.
⚠️ Emeroteca spec still asserts the removed bg-rose-50 nav class
layout.php now marks the active sidebar entry with pk-nav-current, not bg-rose-50. So tests/emeroteca-articles-453-455.spec.js:121 will fail, and the negative check on L122 now passes without testing anything. This file is outside the diff. It belongs to the same sweep as the settings-tab locators fixed in this commit.
Technical details
# Stale active-nav class in emeroteca spec
## Affected sites
- tests/emeroteca-articles-453-455.spec.js:121 — `toHaveClass(/bg-rose-50/)` on the Articoli entry; will time out
- tests/emeroteca-articles-453-455.spec.js:122 — `not.toHaveClass(/bg-rose-50/)` is now always true
## Required outcome
- Both assertions check the class (or `aria-current="page"`) that `app/Views/layout.php:1883-1884` actually sets, so the "only the most specific entry is highlighted" check is meaningful again.claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in these commits. The Desiderata spec synchronisation gap is fixed. The earlier note about
tests/emeroteca-articles-453-455.spec.js:121-122still assertingbg-rose-50is not addressed yet.
Reviewed changes
Reviewed e41596c and 645086e against the prior review at c95ba7a.
- Homepage section order badge: now shows each row's 1-based displayed position, both when the server renders the page (
++$sectionPosition) and after a drag or keyboard reorder (index + 1), instead of the rawdisplay_ordervalue. - Translated Desiderata save in T5: the spec now waits for the success flash after the second submit. The intermediate
gotoalready consumed the first flash, so the message can only appear once the translated POST has completed. That makes it a real sync point beforecontext.close().
php -l app/Views/cms/edit-home.php passes.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found. The last open note from earlier reviews is now fixed.
Reviewed changes
Reviewed 1685de6 against the prior review at 645086e.
- Emeroteca active-nav assertion:
tests/emeroteca-articles-453-455.spec.js:121-122now checksaria-current="page"instead of the removedbg-rose-50class.initializeActiveNavigationinapp/Views/layout.phpsetsaria-currentonly on the link with the longest matchinghref, so the negative check on the/admin/periodicalsentry tests the "most specific entry only" rule again.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed fd5f816 against the prior review at 1685de6.
- Translation menu placement: the new
initTranslationMenus()measures the enclosing.pk-content-language-headingagainstvisualViewport. It opens the "Altre traduzioni" panel on whichever side has more room (data-menu-direction) and caps its height with--translation-menu-space. It refits on scroll and resize, and it closes on an outside click or Escape, returning focus to thesummary. The sizing runs in thesummaryclick handler, before the native toggle shows the panel, and the panel's containing block is the same heading it measures. Both callers (page-header-form.phpand the Desideratacms-section.php) wrap the partial in that heading, so theclosest()lookup always resolves. - Bundle and docs:
main.bundle.jsis rebuilt to match the source, andDESIGN.mddescribes the new behaviour.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
Important
The new series-tree overrides never apply, because the series-tree class is on the wrapper <div> and not on the <table>. The indentation and group-closing rules this commit is meant to restore are still overridden.
Reviewed changes
Reviewed 583d97d against the prior review at fd5f816.
- Series tree overrides: added two rules at the end of
workspace-ui.css. They are meant to restore the 2px group-closing border and the 3rem child indent in the Collane hierarchy, which the generic workspace table rules override.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found. The series-tree selector issue from the previous review is fixed.
Reviewed changes
Reviewed ef1d4f8 against the prior review at 583d97d.
- Series tree overrides now apply: the two rules at the end of
workspace-ui.cssnow select through the.series-treewrapper<div>(.series-tree table tbody …) rather than atable.series-treethat does not exist. At 0,3,6 and 0,3,5 they beat the generictable:not(.fc *) tbody tdrule (0,2,5) and the mobile:is(td, th)padding rule (0,2,4). Child rows get the 3rem indent back, so the text no longer overlaps the connector, and group-closing rows get the 2px border.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed a9f21a7 against the prior review at ef1d4f8.
- Hero cover picker as a combobox:
#hero-cover-searchnow controls arole=listboxofrole=optionbuttons, with arrow-key highlighting througharia-activedescendant. Enter is always prevented, so a search can never submit the CMS form. Escape, Tab and outside clicks close the list. - Stale and failed searches: every
inputevent first callscloseResults(), which clears the debounce timer and aborts the in-flight request. A response only renders if itsAbortControlleris still current, not aborted, and the field still holds the same query. Non-OK and non-array responses now show an error message instead of failing silently, and an empty list or a too-short query also shows feedback. All four new feedback strings exist in every locale. - Browser specs: a new
2bcase inhome-hero-covers-2026.spec.jsroutes/api/search/librito cover the short, empty, error, delayed-then-Escape and keyboard-selection paths.frbr-lrm-crud.spec.jsnow opens the<details>that contains the expression form, not the first one on the page, which is the sidebar's.sweetalert2-popups.spec.jsmounts its fixtures inside#workspace-content.
php -l app/Views/cms/edit-home.php passes.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
Important
The widened shared button rule now sets display: inline-flex with enough specificity to beat Tailwind's hidden class. As a result, buttons that should be hidden on phones, or hidden by JavaScript, stay visible. Separately, a find-and-replace dropped the custom title from the Languages "set default" confirmation dialog.
Reviewed changes
Reviewed 8c76e7a against the prior review at a9f21a7.
- Shared header navigation:
workspace-header.phpputs the back link and breadcrumb on one row. It also renders a defaultHome / <title>breadcrumb when a view supplies none, and now counts/admin/languagesand/admin/cmsas part of the Impostazioni section. - Breadcrumb migration: about 68 per-view breadcrumbs moved to a single
pk-workspace-breadcrumblist with a CSS separator andaria-current="page". - Action geometry: the shared action rule now also covers
.btn-primary/.btn-secondary/.btn-danger*/.btn-iconand several plugin classes. About 74 icon actions gainedbtn-iconvariants andaria-labels. A new block at the end ofworkspace-ui.cssstyles button variants, SweetAlert actions, checkboxes and therole=switchtoggle, the language table and its<progress>meter, the scanner field and the DataTables paging. - Regression spec and notes: the new read-only
tests/workspace-controls-2026.spec.jsanddocs/reviews/admin-controls-2026-10-12.md.
php -l passes on all 101 views that include workspace-header.php.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in this commit. The three threads from the previous review are still open, because this commit does not touch them: the shared button rule overriding
hidden, the lostdata-swal-confirm-titleon the Languages "set default" form, and the eye toggle inplugins.php.
Reviewed changes
Reviewed fd1bbfc against the prior review at 8c76e7a.
- Fallback breadcrumb escaping: the default
Home / <title>crumb inpartials/workspace-header.php:65used to printstrip_tags($workspaceHeader['title'])raw. It now decodes entities and escapes the result again, so it is plain text. Callers passtitlealready escaped (the<h1>at L75 prints it raw), so the decode step prevents double escaping such as&amp;. Any encoded markup in a title comes out escaped, not as live HTML.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in this commit. It fixes the Languages confirmation title (thread resolved). Two threads from the
8c76e7areview are still open because this commit does not touch them: the shared button rule overridinghidden/justify-between, and the eye toggle inplugins.php.
Reviewed changes
Reviewed d8ff742 against the prior review at fd1bbfc. The intervening b06a81c only regenerated the built CSS.
- Set-default confirmation title:
languages/index.php:222getsdata-swal-confirm-titleback, so the dialog shows "Imposta come Predefinita" again rather than the generic fallback. The straytitletooltip on the<form>is gone too. - Square icon padding: the icon rule at
workspace-ui.css:617now usespadding: 0 !important. Its specificity is 0,3,2, so until now the shared action rule (0,3,3) always won and gave these 44px boxespadding: 10px 18px. This means the rule finally takes effect. - Regression check:
workspace-controls-2026.spec.jsnow asserts a computed padding of0pxfor every Languages row action. All six of them are.btn-icon, so the check holds.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found. This commit fixes both remaining threads from the
8c76e7areview: the shared button rule overridinghidden/justify-between, and the API key eye toggle.
Reviewed changes
Reviewed 462e3d3 against the prior review at d8ff742.
- Zero-specificity button layout:
display,justify-contentandtext-alignmoved out of the 0,3,3 shared action rule into a:where(…):not([hidden])rule atworkspace-ui.css:95-98. Tailwind's.hidden,md:inline-flexandjustify-betweensit in the utilities layer, after the compiled.btn-primary/.btn-secondarycomponents at the same 0,1,0 specificity, so they win again. That fixes the duplicate Prestiti actions on phones, the genre Modifica button that would not hide, and the import accordions. - Eye toggle centring: the new
.pk-input-toggleclass keepstranslateY(-50%)at 0,2,2. That beats the compiled.btn-secondary:hoverlift (0,2,0), and the workspace hover/active lift rules now exclude the class. It applies to the core API Book Scraper key field, which also gets the translated "Mostra/nascondi chiave"aria-label(present in all five locales), and to the Discogs token field. - Regression specs:
workspace-controls-2026.spec.jsnow covers the loans actions at 390/767/768/1512px, the genre edit/cancel cycle, accordionspace-between, and the eye staying centred on hover.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed 40370b1 against the prior review at 462e3d3. The commit only touches the staff book record view, a CSS block at the end of workspace-ui.css, and the review notes.
- Header and byline: the media-type badge moves out of the
<h1>into a byline in thecontextslot, next to the author links. Those links replace the "Autori:" row in the left rail and still point to/admin/authors/{id}. The header actions now use the sharedbtn-primary/btn-secondary/btn-danger-outlineclasses. - Missing-cover fallback: an empty or
placeholder.*cover URL, or a failed load (onerrornow setsdata-cover-missinginstead of swapping in the placeholder), shows a typographic cover with escaped title, authors and publisher, plus the "Senza copertina" note. That key exists in all five locales. - Record layout: a 280px cover rail beside the details column, collapsing to one column below 768px. The metadata, description, copies and history cards lose their frames and get serif headings. The status badge carries
data-book-status, and onlydisponibileandin_ritardokeep a coloured background. - Empty copies state:
#physical-copies table:has(tbody td[colspan='7'])hides the header only for the empty-state row. The othercolspan="7"row (loan notes) sits in the loan-history table outside#physical-copies, so it is not affected.
php -l app/Views/libri/scheda_libro.php passes, and every CSS custom property in the new block is defined in interface-tokens.css or workspace-ui.css.
claude-opus-5-5 ｜ 𝕏
# Conflicts: # locale/da_DK.json # locale/de_DE.json # locale/en_US.json # locale/fr_FR.json # locale/it_IT.json
1 participant
Administration and plugin screens had inconsistent headers, action placement and visual styles. On phones, fields were cramped, language tabs overflowed, and long plugin dialogs pushed Save below the screen. This change brings those screens into the existing public frontend’s typography and neutral palette, with restrained theme accents and shared page structure.
Validation:
Production frontend build, ESLint, PHPStan and translation/route/placeholder parity passed.
PHP syntax checked across changed views, controllers, support code and plugins; frontend layout/partials, theme contrast, Dewey validation and CMS language/data-preservation unit checks passed.
Browser review of settings/CMS/plugin screens and dialogs at 320, 390, 844 and 1280px, including landscape: no horizontally clipped controls or JavaScript errors; save actions stay within plugin dialog viewports before and after scrolling.
Verified Dewey language switching, cancellation with unsaved edits, out-of-order loading and disabled saving after a failed load. Browser review submitted no POST requests.
Updated all 23 removed settings-tab locators in four browser suites to a shared helper that uses the actual Sezioni menu. Updated catalogue and Desiderata browser specs for default site-language editing plus optional secondary translation round trips. Two targeted settings browser tests passed locally; translation navigation was checked at 320, 390 and 1280px, and homepage visibility/reordering at 280–1280px. The full database-writing E2E suite was not run locally; it passed in CI on the preceding head. Verified the final series tree CSS in the browser at 280, 320, 390 and 1280px. The complete CI rerun for the final head is in progress.
Cover-picker browser checks passed for title, author and ISBN searches, pointer and keyboard selection, empty/minimum-length feedback and no form submission at desktop and 390px. The browser regressions now scope the expression disclosure to its form and place SweetAlert fixtures inside the scrollable workspace; no forced clicks or hidden controls are used. An additional cover-picker regression exercises keyboard selection, failed/empty requests and a delayed response after Escape. Full-project PHPStan passes.
Validation of the final control changes: all 38 sidebar destinations checked in a real browser at desktop and 390px; 38 additional detail/form URLs on desktop, 10 forms at 390px and 10 representative pages at 320px. Each checked screen has one shared header and breadcrumb and no document overflow. Publisher pagination was exercised through page 2; scanner icon/label alignment, language action targets and core/plugin sidebar weights were measured. Full PHPStan, PHP lint on 83 changed views, 73 behavioral partial checks and the layout suite pass. The new read-only
workspace-controls-2026.spec.jsregression is included automatically in deep E2E CI; checks on the new head are running. Scope and evidence are documented indocs/reviews/admin-controls-2026-10-12.md.Production assets were regenerated with Node 22 after the template class changes. ESLint and the production build pass; a second build produced identical hashes for every tracked asset. This aligns the committed CSS with the frontend and release reproducibility gates.
Staff book record validation: checked at 320, 390, 768 and 1512px with no document overflow, including a real 404 cover fallback and opening/cancelling the physical-copy dialog. PHPStan, PHP syntax, locale parity and existing description/XSS/copy/loan/label checks pass. Protected record IDs, field names, handlers and form actions are preserved. Production build passes with no generated-asset drift. Two additional browser regressions cover hidden/responsive actions, genre editing visibility, accordion layout and input-eye hover centring. Updated-head CI remains in progress.