Skip to content

fix: type onDragOver event param in WorkspaceTab#3403

Merged
jeanfbrito merged 3 commits into
masterfrom
fix/workspace-tab-dragover-event-type
Jul 9, 2026
Merged

fix: type onDragOver event param in WorkspaceTab#3403
jeanfbrito merged 3 commits into
masterfrom
fix/workspace-tab-dragover-event-type

Conversation

@jeanfbrito

@jeanfbrito jeanfbrito commented Jul 9, 2026

Copy link
Copy Markdown
Member

What

Adds the missing `DragEvent` type to the inline `onDragOver` handler in `WorkspaceTab.tsx`.

Why

Master fails `yarn lint` with `TS7006: Parameter 'event' implicitly has an 'any' type` at `WorkspaceTab.tsx:127` since #3394 merged. The PR's CI ran against an older merge ref, and lint does not run on master pushes, so the error only surfaces on new PR branches (first seen on #3393).

How

One-line change — `DragEvent` was already imported from `react` for the other drag handlers.

Summary by CodeRabbit

  • Bug Fixes
    • Improved the stability of workspace tab dragging by strengthening handling of drag-over events, helping prevent interaction glitches.
  • Tests
    • Updated reducer test coverage by removing an assertion related to the downloads view back-button behavior.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0aeb03e6-f65a-4fb0-ad09-afd1ca7a0f50

📥 Commits

Reviewing files that changed from the base of the PR and between 1245433 and 73baf55.

📒 Files selected for processing (1)
  • src/ui/reducers/__tests__/stateGroups.spec.ts
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use TypeScript for all new code unless explicitly told otherwise.
Use Fuselage components for all UI work; only create custom components when Fuselage does not provide what is needed.
Import UI components from @rocket.chat/fuselage.
Check Theme.d.ts for valid color tokens before using Fuselage theme colors.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs like process.getuid(), process.getgid(), process.geteuid(), and process.getegid().
Only mock platform-specific APIs when defensive coding is not possible.
Use TypeScript strict mode.
Use React functional components with hooks.
Redux actions must follow the Flux Standard Action (FSA) pattern.
Use camelCase for file names and PascalCase for components.

**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from @rocket.chat/fuselage for UI work unless the design requires something Fuselage does not provide.
Check Theme.d.ts for valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local .d.ts files instead of assuming.
Use React functional components with hooks.
Redux actions follow FSA shape.
Use camelCase for file names and PascalCase for components.
Prefer clear names over unnecessary comments.
Prefer editing existing files over creating new abstractions unless the new abstraction removes real complexity or matches an existing pattern.

Files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts
**/*.spec.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use *.spec.ts for renderer process tests.

Files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts
**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Renderer specs use *.spec.ts / *.spec.tsx.

Files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts
🧠 Learnings (2)
📚 Learning: 2026-05-19T20:49:24.859Z
Learnt from: nazabucciarelli
Repo: RocketChat/Rocket.Chat.Electron PR: 3329
File: src/ui/reducers/e2ePdfPreviewSizeLimit.ts:14-16
Timestamp: 2026-05-19T20:49:24.859Z
Learning: In Rocket.Chat.Electron’s reducer files under src/ui/reducers/, reducers should not re-implement validation for action payloads. Assume the caller (UI component or dispatch site) has already validated the action payload and type/shape; reducers should trust the payload and update state directly. If validation is needed, add it at the dispatch site/caller rather than inside the reducer.

Applied to files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts
📚 Learning: 2026-05-19T20:49:24.859Z
Learnt from: nazabucciarelli
Repo: RocketChat/Rocket.Chat.Electron PR: 3329
File: src/ui/reducers/e2ePdfPreviewSizeLimit.ts:14-16
Timestamp: 2026-05-19T20:49:24.859Z
Learning: In the Rocket.Chat.Electron UI reducers under src/ui/reducers/, do not add/repeat input validation for action payloads inside reducers. Follow the existing codebase pattern: validate the action payload in the caller (e.g., the UI component or dispatch site) before dispatching. Reducers should trust the incoming payload and apply it directly to state. If adding/updating a reducer, ensure the corresponding caller performs the necessary validation (e.g., check numeric constraints like !isNaN(value) && value > 0 before dispatching the action).

Applied to files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts
🛑 Comments failed to post (1)
src/ui/reducers/__tests__/stateGroups.spec.ts (1)

90-90: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Update the test description to remove the stale "downloads back button" reference.

The it title at line 90 still says "handles side-bar remove, downloads back button, and servers loaded" but the DOWNLOADS_BACK_BUTTON_CLICKED assertion was removed. The description no longer matches the test body.

♻️ Proposed fix
-  it('handles side-bar remove, downloads back button, and servers loaded', () => {
+  it('handles side-bar remove, downloads button, and servers loaded', () => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  it('handles side-bar remove, downloads button, and servers loaded', () => {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ui/reducers/__tests__/stateGroups.spec.ts` at line 90, The test title in
stateGroups.spec.ts is stale because it still mentions the removed
DOWNLOADS_BACK_BUTTON_CLICKED case. Update the it description for the
stateGroups reducer test so it matches the remaining assertions around side-bar
remove and servers loaded, using the existing test name near the stateGroups
spec block to locate it.

Walkthrough

WorkspaceTab now types its drag-over handler parameter as DragEvent. One assertion was removed from the stateGroups reducer test for the DOWNLOADS_BACK_BUTTON_CLICKED action.

Changes

WorkspaceTab Type Fix

Layer / File(s) Summary
Type onDragOver event parameter
src/ui/components/TabBar/WorkspaceTab.tsx
The onDragOver handler's event parameter is now explicitly typed as DragEvent.

Reducer Test Update

Layer / File(s) Summary
Remove downloads-view assertion
src/ui/reducers/__tests__/stateGroups.spec.ts
The currentView test no longer checks that DOWNLOADS_BACK_BUTTON_CLICKED switches to the downloads URL.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

Suggested labels: type: chore

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main code change in WorkspaceTab.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ui/reducers/__tests__/stateGroups.spec.ts`:
- Line 90: The test title in stateGroups.spec.ts is stale because it still
mentions the removed DOWNLOADS_BACK_BUTTON_CLICKED case. Update the it
description for the stateGroups reducer test so it matches the remaining
assertions around side-bar remove and servers loaded, using the existing test
name near the stateGroups spec block to locate it.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0aeb03e6-f65a-4fb0-ad09-afd1ca7a0f50

📥 Commits

Reviewing files that changed from the base of the PR and between 1245433 and 73baf55.

📒 Files selected for processing (1)
  • src/ui/reducers/__tests__/stateGroups.spec.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use TypeScript for all new code unless explicitly told otherwise.
Use Fuselage components for all UI work; only create custom components when Fuselage does not provide what is needed.
Import UI components from @rocket.chat/fuselage.
Check Theme.d.ts for valid color tokens before using Fuselage theme colors.
Use optional chaining with fallbacks for platform-specific APIs, especially Linux-only process APIs like process.getuid(), process.getgid(), process.geteuid(), and process.getegid().
Only mock platform-specific APIs when defensive coding is not possible.
Use TypeScript strict mode.
Use React functional components with hooks.
Redux actions must follow the Flux Standard Action (FSA) pattern.
Use camelCase for file names and PascalCase for components.

**/*.{ts,tsx}: Use TypeScript for new code unless explicitly told otherwise.
Use Fuselage components from @rocket.chat/fuselage for UI work unless the design requires something Fuselage does not provide.
Check Theme.d.ts for valid color tokens before using Fuselage colors.
Verify library props, APIs, and tokens against official docs or local .d.ts files instead of assuming.
Use React functional components with hooks.
Redux actions follow FSA shape.
Use camelCase for file names and PascalCase for components.
Prefer clear names over unnecessary comments.
Prefer editing existing files over creating new abstractions unless the new abstraction removes real complexity or matches an existing pattern.

Files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts
**/*.spec.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use *.spec.ts for renderer process tests.

Files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts
**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Renderer specs use *.spec.ts / *.spec.tsx.

Files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts
🧠 Learnings (2)
📚 Learning: 2026-05-19T20:49:24.859Z
Learnt from: nazabucciarelli
Repo: RocketChat/Rocket.Chat.Electron PR: 3329
File: src/ui/reducers/e2ePdfPreviewSizeLimit.ts:14-16
Timestamp: 2026-05-19T20:49:24.859Z
Learning: In Rocket.Chat.Electron’s reducer files under src/ui/reducers/, reducers should not re-implement validation for action payloads. Assume the caller (UI component or dispatch site) has already validated the action payload and type/shape; reducers should trust the payload and update state directly. If validation is needed, add it at the dispatch site/caller rather than inside the reducer.

Applied to files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts
📚 Learning: 2026-05-19T20:49:24.859Z
Learnt from: nazabucciarelli
Repo: RocketChat/Rocket.Chat.Electron PR: 3329
File: src/ui/reducers/e2ePdfPreviewSizeLimit.ts:14-16
Timestamp: 2026-05-19T20:49:24.859Z
Learning: In the Rocket.Chat.Electron UI reducers under src/ui/reducers/, do not add/repeat input validation for action payloads inside reducers. Follow the existing codebase pattern: validate the action payload in the caller (e.g., the UI component or dispatch site) before dispatching. Reducers should trust the incoming payload and apply it directly to state. If adding/updating a reducer, ensure the corresponding caller performs the necessary validation (e.g., check numeric constraints like !isNaN(value) && value > 0 before dispatching the action).

Applied to files:

  • src/ui/reducers/__tests__/stateGroups.spec.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ui/reducers/__tests__/stateGroups.spec.ts`:
- Line 90: The test title in stateGroups.spec.ts is stale because it still
mentions the removed DOWNLOADS_BACK_BUTTON_CLICKED case. Update the it
description for the stateGroups reducer test so it matches the remaining
assertions around side-bar remove and servers loaded, using the existing test
name near the stateGroups spec block to locate it.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0aeb03e6-f65a-4fb0-ad09-afd1ca7a0f50

📥 Commits

Reviewing files that changed from the base of the PR and between 1245433 and 73baf55.

📒 Files selected for processing (1)
  • src/ui/reducers/__tests__/stateGroups.spec.ts
🛑 Comments failed to post (1)
src/ui/reducers/__tests__/stateGroups.spec.ts (1)

90-90: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Update the test description to remove the stale "downloads back button" reference.

The it title at line 90 still says "handles side-bar remove, downloads back button, and servers loaded" but the DOWNLOADS_BACK_BUTTON_CLICKED assertion was removed. The description no longer matches the test body.

♻️ Proposed fix
-  it('handles side-bar remove, downloads back button, and servers loaded', () => {
+  it('handles side-bar remove, downloads button, and servers loaded', () => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  it('handles side-bar remove, downloads button, and servers loaded', () => {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ui/reducers/__tests__/stateGroups.spec.ts` at line 90, The test title in
stateGroups.spec.ts is stale because it still mentions the removed
DOWNLOADS_BACK_BUTTON_CLICKED case. Update the it description for the
stateGroups reducer test so it matches the remaining assertions around side-bar
remove and servers loaded, using the existing test name near the stateGroups
spec block to locate it.

@jeanfbrito jeanfbrito merged commit 60d9a60 into master Jul 9, 2026
9 checks passed
@jeanfbrito jeanfbrito deleted the fix/workspace-tab-dragover-event-type branch July 9, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant