Skip to content

Update project explorer behavior to prevent focus on revealed items#1045

Merged
kanushka merged 1 commit intowso2:bi-1.5.xfrom
madushajg:main
Dec 5, 2025
Merged

Update project explorer behavior to prevent focus on revealed items#1045
kanushka merged 1 commit intowso2:bi-1.5.xfrom
madushajg:main

Conversation

@madushajg
Copy link
Member

@madushajg madushajg commented Dec 3, 2025

Purpose

Address: wso2/product-ballerina-integrator#2051

Summary by CodeRabbit

  • Improvements
    • Refined project explorer tree view reveal to keep items selected and expanded without shifting keyboard focus.
    • Tightened focus behavior in the data-mapping UI so focus is applied only for sequence-context fields, reducing unexpected focus changes when grouping fields.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

Two small behavior changes: the Project Explorer's TreeView.reveal call now sets focus: false instead of true; the data-mapper's focus handling in processTypeFields is tightened to apply focus logic only when the field is in a sequence context (isSeq true, based on model.groupById).

Changes

Cohort / File(s) Change Summary
Project Explorer — Tree reveal
workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
Changed TreeView.reveal call to use focus: false while keeping select: true and expand: true.
Data-mapper — Focus gating
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts
Tightened condition in processTypeFields: focus-input handling now only runs when isSeq is true (determined by model.groupById), altering when focusMember/focusInputRootMap are applied and when a field is treated as focused.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay attention to the single-line change in project-explorer-provider.ts affecting UI focus behavior.
  • Review conditional logic in processTypeFields for edge cases where model.groupById may be absent or structured differently (possible impacts on focus state propagation).

Possibly related PRs

Suggested reviewers

  • hevayo
  • gigara
  • kanushka

Poem

🐰 I hopped through code at break of dawn,
Two tiny tweaks before the yawn.
Focus softened, sequences guarded tight—
A nibble, a fix, then off in flight! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description only contains a Purpose section with a link to the related issue, but is missing Goals, Approach, Release notes, and other required sections from the template. Complete the PR description by adding Goals, Approach (explaining the implementation and why focus is set to false), Release notes, Documentation, and other applicable sections from the template.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preventing focus on revealed items in the project explorer, which aligns with the modifications in project-explorer-provider.ts.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc26b88 and 56d7f7f.

📒 Files selected for processing (2)
  • workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts (1 hunks)
  • workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 969
File: workspaces/ballerina/data-mapper/src/components/Diagram/utils/node-utils.ts:23-44
Timestamp: 2025-11-20T06:44:07.498Z
Learning: In the data mapper node-utils.ts, when `lastViewIndex == 0`, the code is at the root view and should NOT use `focusInputRootMap` for fallback input node resolution. The truthiness check on `lastViewIndex` is intentional to skip the fallback at the root level.
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:136-141
Timestamp: 2025-11-26T07:49:56.428Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBackButton() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:128-134
Timestamp: 2025-11-26T06:34:09.752Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBreadcrumb() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
📚 Learning: 2025-11-26T06:34:09.752Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:128-134
Timestamp: 2025-11-26T06:34:09.752Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBreadcrumb() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.

Applied to files:

  • workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts
📚 Learning: 2025-11-26T07:49:56.428Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:136-141
Timestamp: 2025-11-26T07:49:56.428Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBackButton() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.

Applied to files:

  • workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts
📚 Learning: 2025-11-20T06:44:07.498Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 969
File: workspaces/ballerina/data-mapper/src/components/Diagram/utils/node-utils.ts:23-44
Timestamp: 2025-11-20T06:44:07.498Z
Learning: In the data mapper node-utils.ts, when `lastViewIndex == 0`, the code is at the root view and should NOT use `focusInputRootMap` for fallback input node resolution. The truthiness check on `lastViewIndex` is intentional to skip the fallback at the root level.

Applied to files:

  • workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts
📚 Learning: 2025-11-24T14:51:49.267Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 998
File: workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx:113-132
Timestamp: 2025-11-24T14:51:49.267Z
Learning: In workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx, if `textFieldRef.current` is not undefined, `textFieldRef.current.inputElement` is guaranteed to exist. If `inputElement` doesn't exist when `current` exists, it's a fatal error that should reach the error boundary rather than being handled with defensive null checks.

Applied to files:

  • workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts
🔇 Additional comments (1)
workspaces/ballerina/ballerina-extension/src/rpc-managers/data-mapper/utils.ts (1)

734-746: The review comment contains a fundamental factual error about the code structure.

The review claims that line 735 adds an isSeq && condition to guard focus handling, but the actual code at line 735 is if (model.focusInputs) with no isSeq guard. Both processTypeFields (line 735) and processArray (line 603) check model.focusInputs identically—neither uses an isSeq check to gate focus member processing.

The isSeq variable is defined but is not used to conditionally access focusInputs. Therefore, there is no inconsistency between the two functions regarding focus handling behavior.

Likely an incorrect or invalid review comment.


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 and usage tips.

@madushajg madushajg changed the base branch from main to bi-1.5.x December 3, 2025 11:14
@madushajg madushajg changed the base branch from bi-1.5.x to main December 3, 2025 11:14
@madushajg madushajg changed the base branch from main to bi-1.5.x December 3, 2025 11:18
@kanushka kanushka merged commit fda97a0 into wso2:bi-1.5.x Dec 5, 2025
6 checks passed
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.

2 participants