Fix namespace popover #172
Merged
Conversation
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
Reviewer's GuideRefactors how publish management is opened from the share popover by moving the PublishManage modal to ShareButton, wiring it through ShareTabs/PublishPanel/PublishLinkPreview, adds a dev-time namespace redirect middleware matching server behavior, tightens server logging and error handling for namespace metadata, adjusts official hostname handling, and extends Cypress tests/selectors for the namespace popover and publish manage flow. Sequence diagram for opening publish manage modal from share popoversequenceDiagram
actor User
participant ShareButton
participant Popover
participant ShareTabs
participant PublishPanel
participant PublishLinkPreview
participant NormalModal
participant PublishManage
User->>ShareButton: click share button
ShareButton->>Popover: open = true
Popover->>ShareTabs: render with opened=true, onOpenPublishManage
ShareTabs->>PublishPanel: render active Panel with onOpenPublishManage
PublishPanel->>PublishLinkPreview: render with onOpenPublishManage
User->>PublishLinkPreview: click open-publish-settings button
PublishLinkPreview->>PublishLinkPreview: onClose()
PublishLinkPreview->>ShareTabs: onOpenPublishManage()
ShareTabs->>ShareButton: call onOpenPublishManage
ShareButton->>Popover: setOpened(false)
ShareButton->>NormalModal: setPublishManageOpen(true)
NormalModal->>PublishManage: render
User->>NormalModal: press Escape key
NormalModal->>ShareButton: onClose()
ShareButton->>NormalModal: setPublishManageOpen(false)
NormalModal-->>PublishManage: unmount
Note over Popover,NormalModal: Share popover is closed before
Note over NormalModal,PublishManage: PublishManage lives outside popover in a top-level modal
Class diagram for updated share and publish management componentsclassDiagram
class ShareButton {
+string viewId
-boolean opened
-boolean publishManageOpen
+ShareButton(props)
}
class ShareTabs {
-boolean opened
-string viewId
-function onClose
-function onOpenPublishManage
-TabKey value
-Array options
+ShareTabs(opened, viewId, onClose, onOpenPublishManage)
}
class PublishPanel {
-string viewId
-boolean opened
-function onClose
-function onOpenPublishManage
+PublishPanel(viewId, opened, onClose, onOpenPublishManage)
}
class PublishLinkPreview {
-string viewId
-object publishInfo
-string url
-boolean loading
-boolean isOwner
-boolean isPublisher
-function onClose
-function onOpenPublishManage
-boolean renameOpen
-string publishName
+PublishLinkPreview(viewId, publishInfo, url, loading, isOwner, isPublisher, onClose, onOpenPublishManage)
}
class PublishManage {
-function onClose
+PublishManage(onClose)
}
class NormalModal {
+boolean open
+function onClose
+string scroll
+boolean overflowHidden
+object okButtonProps
+object cancelButtonProps
+object classes
+ReactNode title
}
ShareButton --> ShareTabs : renders in Popover
ShareButton --> NormalModal : controls open state
NormalModal --> PublishManage : renders as content
ShareTabs --> PublishPanel : Panel option
PublishPanel --> PublishLinkPreview : passes onOpenPublishManage
PublishLinkPreview --> PublishManage : triggers via callback
ShareButton ..> PublishLinkPreview : indirect via props
ShareButton ..> PublishPanel : indirect via Tabs
ShareTabs ..> PublishManage : indirect via callback chain
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
vite.config.ts, consider filtering out falsy entries in thepluginsarray (e.g.plugins: [react(), isDev && namespaceRedirectPlugin(), ...].filter(Boolean)) instead of insertingundefined, as some tooling and typings expect plugins to always be valid objects. - The new Cypress test relies on several fixed
cy.wait(...)calls (including a 5000ms wait); replacing these with waits on specific UI conditions (e.g.should('be.visible')or route aliases) will make the test less flaky and faster.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `vite.config.ts`, consider filtering out falsy entries in the `plugins` array (e.g. `plugins: [react(), isDev && namespaceRedirectPlugin(), ...].filter(Boolean)`) instead of inserting `undefined`, as some tooling and typings expect plugins to always be valid objects.
- The new Cypress test relies on several fixed `cy.wait(...)` calls (including a 5000ms wait); replacing these with waits on specific UI conditions (e.g. `should('be.visible')` or route aliases) will make the test less flaky and faster.
## Individual Comments
### Comment 1
<location> `vite.config.ts:33` </location>
<code_context>
const pathname = url.pathname;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {pathname} = url;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| const url = new URL(req.url, 'http://localhost'); | ||
| const pathname = url.pathname; |
There was a problem hiding this comment.
suggestion (code-quality): Prefer object destructuring when accessing and using properties. (use-object-destructuring)
Suggested change
| const pathname = url.pathname; | |
| const {pathname} = url; |
Explanation
Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.From the Airbnb Javascript Style Guide
josue693
pushed a commit
to josue693/AppFlowy-Web
that referenced
this pull request
Dec 21, 2025
* fix: open publish manage modal after closing share popover * chore: update log
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.
Description
Checklist
General
Testing
Feature-Specific
Summary by Sourcery
Improve publish namespace handling and share popover behavior, and align local dev and deployment behavior for namespace-based routing and logging.
New Features:
Bug Fixes:
Enhancements:
Build:
Deployment:
Tests: