Redirect to previous page when clicking "cancel" on EditFiles page
#1747
Conversation
📝 WalkthroughWalkthroughIntroduces previousUrl storage in EditFiles, updates redirects and cancel navigation to respect SPA mode via FilamentView::hasSpaMode, and uses previousUrl when available, otherwise falls back to the directory listing URL. Error paths now compute ListFiles URL and redirect with SPA awareness. Mount stores url()->previous(). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant EditFiles Page
participant FilamentView
participant Router/Spa
participant ListFiles Page
rect rgb(245,248,255)
note over EditFiles Page: mount()
User->>EditFiles Page: Open file for editing
EditFiles Page->>EditFiles Page: previousUrl = url()->previous()
end
alt Cancel clicked
User->>EditFiles Page: Cancel (Alpine click)
EditFiles Page->>EditFiles Page: target = previousUrl ?? ListFiles::getUrl(dirname(path))
EditFiles Page->>FilamentView: hasSpaMode(target)?
alt SPA mode
EditFiles Page->>Router/Spa: navigate(target)
Router/Spa->>ListFiles Page: Render
else Full page
EditFiles Page->>Router/Spa: redirect(target)
Router/Spa->>ListFiles Page: Reload
end
end
alt Error or Non-editable
EditFiles Page->>EditFiles Page: url = ListFiles::getUrl(dirname(path))
EditFiles Page->>FilamentView: hasSpaMode(url)?
EditFiles Page->>Router/Spa: redirect(url) with SPA awareness
Router/Spa->>ListFiles Page: Navigate
end
alt Save completes
EditFiles Page->>EditFiles Page: url = ListFiles::getUrl(dirname(path))
EditFiles Page->>FilamentView: hasSpaMode(url)?
EditFiles Page->>Router/Spa: redirect(url) with SPA awareness
Router/Spa->>ListFiles Page: Navigate
end
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/Filament/Server/Resources/Files/Pages/EditFiles.php (4)
59-60: Lock the new public property to prevent client-side tampering.Expose it as read-only to Livewire by adding #[Locked].
- public ?string $previousUrl = null; + #[Locked] + public ?string $previousUrl = null;
125-131: Harden cancel navigation and avoid SPA navigate to arbitrary previousUrl.
- Use SPA vs full reload based on the panel’s SPA mode (no argument).
- Prefer history.back() for preserving the search state.
- For the no-referrer fallback, navigate to the listing URL (safe, internal) rather than previousUrl, which could be cross‑origin.
- ->alpineClickHandler(function () { - $url = $this->previousUrl ?? ListFiles::getUrl(['path' => dirname($this->path)]); - - return FilamentView::hasSpaMode($url) - ? 'document.referrer ? window.history.back() : Livewire.navigate(' . Js::from($url) . ')' - : 'document.referrer ? window.history.back() : (window.location.href = ' . Js::from($url) . ')'; - }), + ->alpineClickHandler(function () { + $url = ListFiles::getUrl(['path' => dirname($this->path)]); + + return FilamentView::hasSpaMode() + ? 'document.referrer ? window.history.back() : Livewire.navigate(' . Js::from($url) . ')' + : 'document.referrer ? window.history.back() : (window.location.href = ' . Js::from($url) . ')'; + }),
182-184: Align error redirects with parameterless hasSpaMode().Match the suggested change above for consistency and to avoid potential signature mismatches.
- $this->redirect($url, FilamentView::hasSpaMode($url)); + $this->redirect($url, FilamentView::hasSpaMode());Same diff applies to each occurrence in these catch blocks.
Also applies to: 191-193, 200-202, 205-207
222-223: Sanitize previousUrl to same-origin before storing (optional).Prevents navigating to external origins when no referrer is present, while still enabling history.back() to preserve the search state.
- $this->previousUrl = url()->previous(); + $prev = url()->previous(); + if (parse_url($prev, PHP_URL_HOST) !== request()->getHost()) { + $prev = null; + } + $this->previousUrl = $prev;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Filament/Server/Resources/Files/Pages/EditFiles.php(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/EditFiles.php (2)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
ListFiles(49-601)app/Livewire/AlertBanner.php (2)
AlertBanner(14-100)closable(72-77)
🔇 Additional comments (3)
app/Filament/Server/Resources/Files/Pages/EditFiles.php (3)
31-31: Import looks correct.FilamentView facade is appropriate for SPA checks.
37-37: Good choice using Js::from for safe JS embedding.
100-102: Don't pass $url into FilamentView::hasSpaMode — verify the method signatureConfirm the signature in the installed Filament package (vendor/) or upstream; if it's parameterless, replace FilamentView::hasSpaMode($url) with FilamentView::hasSpaMode() to avoid an ArgumentCountError.
Occurrences to update: app/Filament/Server/Resources/Files/Pages/EditFiles.php — lines 101, 128, 183, 192, 201, 206.
Example replacement:
- $this->redirect($url, FilamentView::hasSpaMode());
- return FilamentView::hasSpaMode() ? 'document.referrer ? window.history.back() : Livewire.navigate(' . Js::from($url) . ')' : 'document.referrer ? window.history.back() : (window.location.href = ' . Js::from($url) . ')';
Closes #1743
Copied from filaments cancel action.