Give staff hub Create and View distinct compose and list URLs - #209
Conversation
Staff could not tell create from browse because both hub actions used the same index route, and the Pet Profiles label implied /shelter/pet-profiles which 404s. Co-authored-by: APES Director <apescic-director@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This is the first Staff UX ship, so the hub navigation fix opens the 0.33.x line in the same PR as the work. Co-authored-by: APES Director <apescic-director@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab71cf2ebf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public function viewUrl(): string | ||
| { | ||
| return route($this->routeName).'#list'; |
There was a problem hiding this comment.
Add the required release metadata
This feature commit leaves VERSION, resources/data/releases.json, resources/data/module-runtime-contract.json, and the version-pinned tests unchanged from its main parent. On the pull request, .github/workflows/test-cloudron.yml runs myapes:changelog-validate against the base SHA, so validateAppendOnly() will report that exactly one release record must be prepended and block CI. Include the synchronized v0.33.0 release metadata in this change.
AGENTS.md reference: AGENTS.md:L87-L89
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
v0.33.0 metadata is already on this PR in 20837ba (VERSION, releases.json, module-runtime-contract.json, and the version-pinned tests). CI on that commit was green. The review was against the earlier feature-only commit ab71cf2.
|
|
||
| public function viewUrl(): string | ||
| { | ||
| return route($this->routeName).'#list'; |
There was a problem hiding this comment.
Offset fragment targets below the mobile header
On viewports at or below 64rem, .mobile-header is sticky with a minimum height of 4.5rem, while the newly targeted #list and #create panels have no scroll-margin-top. Following either hub link therefore aligns the panel with the top of the viewport underneath the header, obscuring its heading and the first part of the requested section. Add an appropriate scroll offset to these fragment targets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
#create and #list now have scroll-margin-top: calc(4.5rem + 0.75rem) at the mobile-header breakpoint so the heading clears the sticky bar.
| </div> | ||
| @endif | ||
| <div class="panel"> | ||
| <div class="panel" id="list"> |
There was a problem hiding this comment.
Preserve the list target across pagination
When any of these lists contains more than 20 records, Laravel's generated pagination links contain the page query but not the new #list fragment. A staff member who arrives via the View link and selects another page is therefore returned to the top of the index; if they can create records, the compose form appears before the requested list again. Set the paginator's fragment to list on all six affected index endpoints so browsing remains anchored to the list.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Service index paginators now use fragment('list'), so page links keep #list. Covered for Shelter pets and APES CIC tickets in ServiceHubQuickLinksTest.
Pagination links dropped #list, and the sticky mobile header covered the compose and list headings. Co-authored-by: APES Director <apescic-director@users.noreply.github.com>
Fixes #136
Staff service hubs showed Create and View in “what you can do” / Quick links, but both used the same index URL. Staff could not tell compose from browse. The Shelter Pet Profiles label also implied
/shelter/pet-profiles, which 404s; the live list is/shelter/pets.What this changes
#list)#create)/shelter/pets#list, not/shelter/pet-profiles/shelter/pet-profilesand/petcare/pet-profilesredirect to the live pets list#listso page 2+ stays on the list#create/#listuse a scroll offset under the sticky mobile headerOut of scope
/superadmin/pluginsand/superadmin/groups([v1.0.0] 10 [Bug] Stale/superadmin/pluginsand/superadmin/groups404 #133)Tests
ServiceHubQuickLinksTestpins Create ≠ View URLs, Shelter pets path, alias redirects, hidden Create without permission, and pagination#listModuleAdministrationAndNavigationTestnow asserts the Shelter pets list route on the hubModuleStateMiddlewareTestcovers the pet-profiles alias routesphp artisan myapes:changelog-validate --base-ref=origin/mainpassed on the PR branchnpm run test:frontendandnpm run buildpassedcomposer pre-pr-verifywas not run as the Composer script (PowerShell). The Linux equivalent contract gate above was run instead. Fullbootstrap.sh --seedand a live browser preview were not started in this environment; verification is route- and test-based.Release metadata
This PR includes v0.33.0 (
VERSION,resources/data/releases.json,resources/data/module-runtime-contract.json). This is the first Staff UX ship (minor).