fix: Resolve issues of new file viewer UI 🐛

[lethemanh]

Related to:

linagora/twake-drive#3691

Changes:

This PR resolves these issues of new UI of file viewer:

1. Cannot open the details after clicking on Share
2. Summarization panel is displaying same time with Detail panel
3. Make it shorter and Add more detail are the actions to change the summarization by AI
4. Hide antivirus block if there is no result
5. Add missing translation key

[coderabbitai]

Walkthrough

Replaces the legacy expand/shorter actions with makeSummaryLonger/makeSummaryShorter, changing those actions to use router navigate/location and set showAIAssistant and an action type in location state. Adds LONG_SUMMARY_SYSTEM_PROMPT and SHORT_SUMMARY_SYSTEM_PROMPT and selects the system prompt in AIAssistantPanel based on location.state.type. Adjusts summary fetching and callbacks to include location, changes assistant-close navigation to the current pathname, forwards currentIndex to ViewerInformationsWrapper and adds effects driving the file viewer panel, updates getPanelBlocks to accept t and uses getAntivirusStatus, and adds a new panel.collapse locale key.

Possibly related PRs

• feat: Implement new UI of file viewer ✨ #2914: Overlapping edits to the AI assistant panel and related action/export changes touching AIAssistantPanel.jsx and prompts.
• feat(cozy-viewer): Add antivirus panel block #2907: Adds getAntivirusStatus and antivirus panel logic; this change imports and uses that helper in panel block conditions.

Suggested reviewers

• JF-Cozy
• rezk2ll
• doubleface
• zatteo

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Resolve issues of new file viewer UI' broadly describes the changeset, but is vague about the specific changes made. The PR includes multiple distinct changes: refactoring summary actions (makeSummaryLonger/makeShorter), adding AI assistant prompt variants, updating panel logic, and localization additions. The title does not convey the main technical nature of these changes.	Consider a more specific title that highlights the primary change, such as 'refactor: Migrate summary actions to router-based state management' or breaking into multiple PRs if changes serve different purposes.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-file-viewer-ui

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/cozy-viewer/src/ViewerInformationsWrapper.jsx (1)

99-107: ⚠️ Potential issue | 🟡 Minor

Missing PropType for currentIndex.

The currentIndex prop is used (line 21) and included in effect dependencies (line 48) but is not declared in PropTypes.

🔧 Proposed fix

 ViewerInformationsWrapper.propTypes = {
   disableFooter: PropTypes.bool,
   validForPanel: PropTypes.bool,
   toolbarRef: PropTypes.object,
+  currentIndex: PropTypes.number,
   children: PropTypes.oneOfType([
     PropTypes.node,
     PropTypes.arrayOf(PropTypes.node)
   ])
 }

🧹 Nitpick comments (2)

packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)

174-174: Consider narrowing the dependency to avoid unnecessary re-fetches.

Adding the entire location object to the dependency array will trigger fetchSummary recreation (and thus a re-fetch via the useEffect on line 189-191) whenever any property of location changes—not just type. This could cause unintended API calls if other state properties or pathname change.

Consider extracting only the relevant value:

♻️ Suggested approach

+ const summaryType = location.state?.type
+
  const fetchSummary = useCallback(
    async (force = false) => {
      // ... existing logic
    },
-   [client, file, t, location]
+   [client, file, t, summaryType]
  )

Then update summarizeFile to accept summaryType as a parameter instead of reading from location.state directly.

packages/cozy-viewer/src/ViewerInformationsWrapper.jsx (1)

44-48: Clarify the purpose of currentIndex in the dependency array.

The effect ensures the file viewer panel opens when the AI assistant closes. Including currentIndex means the effect also runs when switching files—even if isOpenAiAssistant hasn't changed. While calling setIsOpenFileViewerPanel(true) when the panel is already open is harmless, consider adding a comment to clarify the intent, or conditionally check before setting:

💡 Optional: Add guard or comment

  useEffect(() => {
    if (!isOpenAiAssistant) {
+     // Ensure panel is visible when AI assistant closes or when switching files
      setIsOpenFileViewerPanel(true)
    }
  }, [isOpenAiAssistant, currentIndex, setIsOpenFileViewerPanel])

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

packages/cozy-viewer/src/ViewerInformationsWrapper.jsx (1)

99-107: ⚠️ Potential issue | 🟡 Minor

Missing currentIndex in PropTypes.

The currentIndex prop is destructured and used in the component but is not defined in PropTypes.

📝 Proposed fix

 ViewerInformationsWrapper.propTypes = {
   disableFooter: PropTypes.bool,
   validForPanel: PropTypes.bool,
   toolbarRef: PropTypes.object,
+  currentIndex: PropTypes.number,
   children: PropTypes.oneOfType([
     PropTypes.node,
     PropTypes.arrayOf(PropTypes.node)
   ])
 }

packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)

284-294: ⚠️ Potential issue | 🟡 Minor

PropTypes and defaultProps are stale and don't match actual usage.

The component only receives className as a prop (line 34), but propTypes declare isLoading, summary, onStop, and onSend which are not used. The defaultProps also define values for internal state variables.

🔧 Proposed fix

 AIAssistantPanel.propTypes = {
-  isLoading: PropTypes.bool,
-  summary: PropTypes.string,
-  onStop: PropTypes.func,
-  onSend: PropTypes.func
+  className: PropTypes.string
 }

-AIAssistantPanel.defaultProps = {
-  isLoading: false,
-  summary: ''
-}

🧹 Nitpick comments (4)

packages/cozy-viewer/src/ViewerInformationsWrapper.jsx (1)

44-48: Consider adding a comment to clarify the currentIndex dependency.

The currentIndex appears in the dependency array but is not used in the effect body. This is intentional—it ensures the file viewer panel reopens when navigating between files—but this pattern can confuse future maintainers who may see it as an unnecessary dependency or a linting oversight.

📝 Suggested clarification

  useEffect(() => {
+   // When AI assistant closes OR user navigates to a different file,
+   // ensure the file viewer panel is open
    if (!isOpenAiAssistant) {
      setIsOpenFileViewerPanel(true)
    }
  }, [isOpenAiAssistant, currentIndex, setIsOpenFileViewerPanel])

packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)

57-104: Potential stale closure: summarizeFile reads location.state but isn't memoized.

The summarizeFile function (lines 79-84) reads location.state.type to select the system prompt, but it's defined as a regular function inside the component. When fetchSummary calls summarizeFile, if location changes between renders, the closure may reference stale state.

Consider either:

1. Moving the prompt selection logic into fetchSummary (which has location in its deps), or
2. Passing location.state.type as a parameter to summarizeFile

♻️ Suggested refactor: Pass type as parameter

- const summarizeFile = async ({ client, file, stream = false, model }) => {
+ const summarizeFile = async ({ client, file, stream = false, model, summaryType }) => {
    try {
      // ... extractText logic unchanged ...

      let systemPrompt = SUMMARY_SYSTEM_PROMPT
-     if (location.state?.type === 'makeSummaryLonger') {
+     if (summaryType === 'makeSummaryLonger') {
        systemPrompt = LONG_SUMMARY_SYSTEM_PROMPT
-     } else if (location.state?.type === 'makeSummaryShorter') {
+     } else if (summaryType === 'makeSummaryShorter') {
        systemPrompt = SHORT_SUMMARY_SYSTEM_PROMPT
      }

Then in fetchSummary:

- const response = await summarizeFile({ client, file, stream: false })
+ const response = await summarizeFile({ client, file, stream: false, summaryType: location.state?.type })

packages/cozy-viewer/src/Panel/getPanelBlocks.jsx (1)

91-98: Update JSDoc to document the new t parameter.

The function signature now includes t (translation function), but the JSDoc at lines 91-97 doesn't document it.

📝 Suggested documentation update

 /**
  * Returns the blocks to display in the panel
  * `@param` {Object} options
  * `@param` {PanelBlocksSpecs} options.panelBlocksSpecs - Specs of the blocks to display in the panel
  * `@param` {import('cozy-client/types/types').FileDocument} options.file - File object
+ * `@param` {Function} options.t - Translation function
  * `@returns` {Array.<React.Component>}
  */
 const getPanelBlocks = ({ panelBlocksSpecs, file, t }) => {

packages/cozy-viewer/src/actions/makeSummaryLonger.js (1)

26-26: Minor inconsistency: Parameter order differs from makeSummaryShorter.

This function uses ({ t, navigate, location }) while makeSummaryShorter uses ({ t, location, navigate }). While this works due to destructuring, consistent ordering improves readability.

♻️ Suggested fix for consistency

-export const makeSummaryLonger = ({ t, navigate, location }) => {
+export const makeSummaryLonger = ({ t, location, navigate }) => {

[zatteo]

When you fix multiple bugs, please create one commit per bug to ease understanding and future retrieval of bug fix reason

[zatteo]

Much better thank you!