[Fix] Fix Various bugs on the Explore Apps Page#1142
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughClient modal open state is derived from the pathname; per-project app enabled state is read from config; handlers added for open, open-change, and disable that navigate with router.replace; Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant Modal
participant Config
participant AppEntry
User->>Router: navigate to modal route (/projects/:id/@modal/apps/:appId)
Router->>Modal: render modal (isOpen derived from pathname)
Modal->>Config: read project apps config (apps.installed[appId].enabled)
Config-->>Modal: return isEnabled
Modal->>AppEntry: render with isEnabled, onOpen, onDisable
User->>AppEntry: click "Open"
AppEntry->>Modal: call onOpen()
Modal->>Router: router.replace to app frontend path (guarded)
User->>Modal: close modal
Modal->>Router: router.replace back to apps list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Contributor
Greptile OverviewGreptile SummaryThis PR fixes modal state management issues on the explore apps page. The main changes separate navigation from the enable action, dynamically track modal state, and coordinate pathname changes with navigation intent. Key improvements:
The implementation uses two Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Modal as AppDetailsModal
participant Router
participant Config as ConfigUpdate
participant AppStore as AppStoreEntry
User->>Modal: Opens app details modal
Modal->>Modal: setIsOpen(true)
Modal->>Modal: pathname effect triggers
Modal->>AppStore: Renders with isEnabled status
alt App Not Enabled
User->>AppStore: Clicks Enable App
AppStore->>Modal: handleEnable()
Modal->>Config: updateConfig()
Config-->>Modal: Success
Note over Modal: isEnabled updates via config hook
AppStore->>AppStore: Re-renders showing Open App button
end
alt App Enabled
User->>AppStore: Clicks Open App
AppStore->>Modal: handleOpen()
Modal->>Modal: hasNavigatedRef = false
Modal->>Modal: setNavigateTo(path)
Modal->>Modal: setIsOpen(false)
Modal->>Modal: Navigation effect triggers
Modal->>Router: router.replace(navigateTo)
end
alt User Closes Modal
User->>Modal: Closes modal
Modal->>Modal: handleOpenChange(false)
Modal->>Modal: setIsOpen(false)
alt No pending navigation
Modal->>Router: router.replace to apps list
end
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple bugs on the explore apps page, specifically addressing issues where clicking "Enable App" would show the button even after the app was enabled, and preventing unnecessary redirects while the modal is still open.
Changes:
- Implemented dynamic modal state management using
isOpen,navigateTo, andhasNavigatedRefto control modal visibility and navigation timing - Added pathname tracking to properly handle modal route transitions and prevent stale navigation attempts
- Removed the artificial 1-second delay (
wait(1000)) from the enable app flow and improved the navigation logic to allow users to open the app page directly from the modal after enabling
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nams1570
force-pushed
the
explore-apps-bugs
branch
from
b612091 to
812d73b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/@modal/(.)apps/[appId]/page-client.tsx:
- Around line 62-76: The modal is being closed immediately before navigation can
be confirmed, which can leave the UI on the modal route with the modal hidden;
update handleOpen and handleOpenChange to start navigation but not call
setIsOpen(false) until the pathname actually changes: remove the immediate
setIsOpen(false) in handleOpen, and in handleOpenChange when open becomes false
do router.replace(`/projects/${project.id}/apps`) (or setNavigateTo) without
closing the modal; rely on the existing pathname effect (which should compare
the current pathname to the modal route and use hasNavigatedRef) to
setIsOpen(false) only after the router pathname successfully changes, and ensure
hasNavigatedRef.current is reset appropriately so retries are possible if
navigation is vetoed.
🧹 Nitpick comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/@modal/(.)apps/[appId]/page-client.tsx (1)
74-75: UseencodeURIComponent(orurlString) for the base apps path.
The project id is interpolated directly into a URL string; please encode it for consistency and safety.🔧 Proposed tweak
- router.replace(`/projects/${project.id}/apps`); + router.replace(`/projects/${encodeURIComponent(project.id)}/apps`);As per coding guidelines: Use
urlStringtemplate literals orencodeURIComponent()instead of normal string interpolation for URLs, for consistency.
N2D4
approved these changes
Feb 2, 2026
Before, an app on explore app would show enable app even when enabled. Also, the open page button behavior was buggy. We now prevent random redirects behind the modal.
isOpen was sued to render modal. It was never reset to true. However, pathname still updated on clicking the appentry. So we use a change in pathname to update isOpen. We check to make sure that pathname change is to an app entry, else modal would reopen randomly on going to the base exporeapps page too.
This deals with issues in our old approach. If a confirmation dialog pops up, and the user declines to navigate, the modal will not be closed.
nams1570
force-pushed
the
explore-apps-bugs
branch
from
e5d38b5 to
731db26
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
There are a smattering of bugs on the explore apps page. Clicking "Enable App" may enable the app, but the button still shows, plus it causes unnecessary redirects while the modal is still up. This behavior can be seen in the linked clip
Explore.Apps.before.mov
Summary of Changes
We dynamically handle the modal open state, and track the path updates. This lets us deal with the bugs above while avoiding unnecessary renders, allowing reopening of previously opened modals, and preventing unnecessary redirects.
Dealing with the enable apps button issues also now allows users to navigate to the app page from the explore apps modal.
We also add a disable button to the modal. Previously, users had to check the options for each app in order to disable it. Now they can do it on the modal itself, which is in line with how the "Enable App" functionality works.
UI Demo
Explore.Apps.After.with.disable.mov
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Style