Skip to content

fix: Whether to display a share button on the conversation page - #7818

Merged
shaohuzhang1 merged 1 commit into
v3from
pr@v3@fix_show_share
Nov 10, 2026
Merged

shaohuzhang1 merged 1 commit into
v3from
pr@v3@fix_show_share

Conversation

@shaohuzhang1

Copy link
Copy Markdown
Contributor

fix: Whether to display a share button on the conversation page

<ChatHeader />
</slot>
<!-- 聊天背景铺满标题栏以下的消息区与输入区,与显示设置预览一致 -->
<div

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is an analysis of the provided code diff:

1. Irregularities & Potential Issues

  • Breaking Change (API Reduction): The shareable prop and share event have been completely removed from this component's interface. If any parent components are still passing :shareable="true" or listening for @share, they will likely cause Vue warnings (unknown props/events) or fail to trigger sharing functionality.
  • Lost Functionality: Unless the ChatHeader component now handles shareability logic internally independently of external props, the ability to control share visibility from outside this component is lost. Verify if ChatHeader has hard-coded default behavior for sharing that satisfies all previous use cases.

2. Optimization Suggestions

  • Check Parent Components: Ensure all callsites of this component have been updated to remove references to shareable and @share.
  • Review ChatHeader: Confirm that ChatHeader still supports sharing in scenarios where it was previously explicitly enabled/disabled by the parent. If sharing should always be hidden by default, this change simplifies the API. If it should be contextual, consider restoring the prop prop or moving the state to a Pinia store/Vuex module if global access is needed.

Summary

The code cleanup simplifies the component signature but is a breaking change. It is safe only if no external consumer relies on the removed shareable prop or share event.

<template #main><ChatPanel /></template>
<template #right><ExecutionDetail /></template>
</ConversationLayout>
</template>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the review of the code changes:

1. Potential Issues

  • Removal of Static Prop: You removed shareable from <ChatPanel />. Ensure that ChatPanel and its child components (title bar, menu, etc.) are now strictly relying on the injected SHOW_SHARE_KEY computed property for their visibility logic. If any part of ChatPanel still expects the shareable prop or fails to fall back correctly when it's missing, UI elements might disappear unexpectedly.
  • Reactivity Dependency: The computed property depends on bundle.list.appInfo.value?.show_share. Ensure that:
    1. bundle.list.appInfo is a reactive ref.
    2. Updating appInfo triggers the re-computation of this provided value.
    3. Consumers of SHOW_SHARE_KEY use inject and access .value correctly if injecting as a ref/computed.

2. Optimization Suggestions

  • Null Check Consistency: The code uses optional chaining (?.) which is good. However, Boolean(...) is generally preferred over !! for readability, which you have done. No change needed here, just confirming it's a best practice.
  • Type Safety: Ensure show_share is expected to be a boolean or numeric/falsy truthy value in your type definition for appInfo. If it's strictly boolean | undefined, Boolean() handles the undefined case correctly (returns false).

3. Style & Best Practices

  • Comments: The added Chinese comment is clear and helpful for context maintenance. Consider translating it to English if your codebase requires English comments for consistency.
  • Formatting: The provide call is well-formatted across multiple lines. This is clean.

4. Summary

The change effectively shifts the "shareability" logic from a static, per-component prop to a dynamic, app-level contextual state via Vue's provide/inject. This is a superior architectural choice if share permissions depend on global configuration (appInfo) rather than local component instance states.

Action Required: Verify that all components previously relying on the shareable prop on ChatPanel have been updated to inject(SHOW_SHARE_KEY).

const showShare = useShowShare()
const shareLink = useShareLinkStoreOptional()
const shareRecordId = useShareRecordId()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code modifications introduce several logical issues and anti-patterns:

1. Violation of Vue Composition API Rules (Critical)
const showShare = useShowShare() executes useShowShare() immediately during the script setup phase. If useShowShare is a standard composable returning a reactive reference (e.g., Ref<boolean>), assigning it directly to a plain const means:

  • It might return a Ref object, not a boolean value, causing template checks (v-if="showShare") to always be truthy.
  • Even if it returns a primitive, it loses reactivity unless the composable itself manages complex internal state differently than standard patterns.
    Fix: Ensure useShowShare returns a reactive ref and use it correctly in the template, or destructure/compute if necessary. If it's a computed property inside, useShowShare should likely return a ComputedRef.

2. Reactivity Loss
Previously, SHAREABLE_VIEWS.includes(useViewType()) suggests useViewType() is a reactive getter or function call that was potentially evaluated once but relied on imported constants. Now, replacing it with a custom hook useShowShare() isolates logic appropriately, but you must ensure useShowShare properly tracks dependencies (like route params or view types) so that showShare updates dynamically when the user switches views.

3. Comment Alignment
The comment text updated from "views sharing service" to "agent allows sharing". Ensure the business logic inside useShowShare matches this new description.

4. Redundant Imports Check
Verify that useViewType is truly not used elsewhere in this file since it was removed. Similarly, ensure SHAREABLE_VIEWS is no longer needed.

Optimization Suggestion:
If useShowShare is simply a boolean check based on current route/view, consider if it could be derived directly via computed within this component for better locality, unless it is reused across multiple components (which justifies a separate hook).

@shaohuzhang1
shaohuzhang1 merged commit b22225d into v3 Nov 10, 2026
3 of 4 checks passed
@shaohuzhang1
shaohuzhang1 deleted the pr@v3@fix_show_share branch November 10, 2026 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant