IEP-1345: Fix for refreshing the manager after installation #1098
Conversation
WalkthroughThe pull request introduces modifications to the ESP-IDF UI plugin, focusing on implementing a singleton pattern for the Changes
Sequence DiagramsequenceDiagram
participant Startup as InitializeToolsStartup
participant Job as ToolsActivationJob
participant Listener as ToolsActivationJobListener
participant Page as ESPIDFMainTablePage
Startup->>Job: Create Job
Startup->>Listener: Create Listener
Startup->>Job: Add Job Change Listener
Job-->>Listener: Notify Job State Changes
Listener->>Page: Update UI or Trigger Refresh
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java (1)
Line range hint
275-284: Consider extracting button state management logicThe removeAllButton state management logic is duplicated in createIdfTable and refreshEditorUI methods.
Consider extracting the logic into a private method:
+private void updateRemoveAllButtonState() { + if (idfToolSets == null || idfToolSets.isEmpty()) { + removeAllButton.setEnabled(false); + } else { + removeAllButton.setEnabled(true); + } +}Then replace the duplicate code in both methods with:
-if (idfToolSets == null || idfToolSets.isEmpty()) { - removeAllButton.setEnabled(false); -} else { - removeAllButton.setEnabled(true); -} +updateRemoveAllButtonState();Also applies to: 141-150
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/InitializeToolsStartup.java(2 hunks)bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/ESPIDFManagerEditor.java(1 hunks)bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java(2 hunks)
🔇 Additional comments (4)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/ESPIDFManagerEditor.java (1)
56-57: LGTM: Proper usage of singleton pattern
The change correctly uses the singleton pattern to maintain a single instance of ESPIDFMainTablePage across the application.
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/InitializeToolsStartup.java (1)
185-186: LGTM: Proper implementation of job listener
The changes correctly implement job state tracking by:
- Creating a job listener with the singleton instance of ESPIDFMainTablePage
- Adding the listener to track tool activation job state changes
This implementation ensures proper UI refresh after tool installation.
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/tools/manager/pages/ESPIDFMainTablePage.java (2)
70-84: LGTM: Clean singleton pattern implementation
The singleton pattern is correctly implemented with:
- Private static instance
- Private constructor
- Public static getInstance() method
100-101: LGTM: Proper null check added
The null check for container prevents potential NullPointerException if refreshEditorUI is called before the UI is created.
|
@alirana01 hi ! Tested : OS - Windows 11 ESP-IDF Manager shell refreshed after tools installation - tools are displayed ✅ LGTM 👍 |
Description
ESP-IDF Manager: refresh page after tools installed.
Fixes # (IEP-1345)
Type of change
Please delete options that are not relevant.
How has this been tested?
Please use the esp-idf.json from the offline installer in the root directory of the ide to test this and verify the tools installation normally as well
Test Configuration:
Checklist
Summary by CodeRabbit
New Features
ESPIDFMainTablePage, ensuring a single instance is used throughout the application.Bug Fixes
Refactor
ESPIDFMainTablePageto utilize a singleton instance rather than creating new instances.