Skip to content

fix: 修复全选能选中不支持当前控制器的任务#142

Merged
MistEO merged 3 commits intoMistEO:mainfrom
ShadowLemoon:fix/select-all
Apr 3, 2026
Merged

fix: 修复全选能选中不支持当前控制器的任务#142
MistEO merged 3 commits intoMistEO:mainfrom
ShadowLemoon:fix/select-all

Conversation

@ShadowLemoon
Copy link
Copy Markdown
Contributor

@ShadowLemoon ShadowLemoon commented Mar 31, 2026

Summary by Sourcery

确保在切换所有任务时,任务的批量选择会遵循控制器和资源兼容性。

Bug 修复:

  • 修复全选行为,使其不会启用与当前选定控制器或资源不兼容的任务。

增强功能:

  • 使工具栏的“全部已选中”状态计算方式与任务的控制器/资源兼容性约束保持一致。
Original summary in English

Summary by Sourcery

Ensure task bulk selection respects controller and resource compatibility when toggling all tasks.

Bug Fixes:

  • Fix select-all behavior so it does not enable tasks that are incompatible with the currently selected controller or resource.

Enhancements:

  • Align the toolbar’s all-selected state calculation with controller/resource compatibility constraints for tasks.

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了 1 个问题,并给出了一些整体性的反馈:

  • selectAllTasksToolbar 中关于 controller/resource 的兼容性检查是重复的,而且存在细微差异;建议抽取一个共享的工具函数(例如 isTaskCompatible(taskDef, controllerName, resourceName))来保持逻辑一致,并且更易维护。
  • allEnableduseMemo 中,将 projectInterface?.task 放在依赖数组里可能不满足 hook 的 lint 规则,而且一旦 projectInterface 的引用发生变化会比较脆弱;更安全的做法是依赖 projectInterface(或某个稳定的派生值)。
给 AI 代理的提示
请根据下面的代码评审评论进行修改:

## 总体评论
- `selectAllTasks``Toolbar` 中关于 controller/resource 的兼容性检查是重复的,而且存在细微差异;建议抽取一个共享的工具函数(例如 `isTaskCompatible(taskDef, controllerName, resourceName)`)来保持逻辑一致,并且更易维护。
-`allEnabled``useMemo` 中,将 `projectInterface?.task` 放在依赖数组里可能不满足 hook 的 lint 规则,而且一旦 `projectInterface` 的引用发生变化会比较脆弱;更安全的做法是依赖 `projectInterface`(或某个稳定的派生值)。

## 逐条评论

### 评论 1
<location path="src/stores/appStore.ts" line_range="618-619" />
<code_context>
   // 检查是否有保存的设备和资源配置(用于权限检查等)
   const currentControllerName =
     selectedController[instanceId] || projectInterface?.controller[0]?.name;
+  const currentResourceName =
</code_context>
<issue_to_address>
**suggestion:** controller/resource 的解析逻辑与 Toolbar 中的逻辑重复,可以进行集中抽取。

这里的逻辑与 `Toolbar` 中的解析逻辑是镜像关系(包括回退顺序和 `projectInterface` 的使用)。建议抽取一个共享工具函数(例如 `getCurrentControllerAndResource(state, instanceId)`),这样规则可以保持一致,而且只需要在一个地方更新。

推荐实现:

```typescript
export const getCurrentControllerAndResource = (state: AppState, instanceId: string) => {
        const pi = state.projectInterface;
        const instance = state.instances.find((i) => i.id === instanceId);

        const controllerName =
          state.selectedController[instanceId] ||
          instance?.controllerName ||
          pi?.controller[0]?.name;

        const resourceName =
          state.selectedResource[instanceId] ||
          instance?.resourceName ||
          pi?.resource[0]?.name;

        return { controllerName, resourceName };
      };

      set((state) => {

```

```typescript
        const { controllerName: currentControllerName, resourceName: currentResourceName } =
          getCurrentControllerAndResource(state, instanceId);

```

1. 在 `Toolbar` 组件中(或当前存在重复 controller/resource 解析逻辑的任何地方),用对 `getCurrentControllerAndResource(state, instanceId)` 的调用替换内联逻辑,以保持行为一致。
2. 确保在该工具函数的签名中,`AppState`(或正确的 store state 类型)已经被导入或在作用域内;如果类型名不同,请将 `AppState` 调整为 `appStore.ts` 中实际使用的类型。
3. 从 `src/stores/appStore.ts` 中导出 `getCurrentControllerAndResource`(如果你的打包器/TS 设置要求在声明之外显式导出),并在 `Toolbar` 中导入它。
4. 如果 `Toolbar` 无法直接访问原始的 store state,你可能需要一个小的包装 selector,通过 store 调用 `getCurrentControllerAndResource`(例如:`useAppStore((state) => getCurrentControllerAndResource(state, instanceId)))`
</issue_to_address>

Sourcery 对开源项目是免费的 —— 如果你觉得这些评审有帮助,欢迎分享 ✨
帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English

Hey - I've found 1 issue, and left some high level feedback:

  • The compatibility checks for controller/resource in selectAllTasks and Toolbar are duplicated and subtly different; consider extracting a shared helper (e.g. isTaskCompatible(taskDef, controllerName, resourceName)) to keep the logic consistent and easier to maintain.
  • In the useMemo for allEnabled, using projectInterface?.task in the dependency array may not satisfy hook lint rules and can be fragile if projectInterface changes identity; it’s safer to depend on projectInterface (or a stable derived value) instead.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The compatibility checks for controller/resource in `selectAllTasks` and `Toolbar` are duplicated and subtly different; consider extracting a shared helper (e.g. `isTaskCompatible(taskDef, controllerName, resourceName)`) to keep the logic consistent and easier to maintain.
- In the `useMemo` for `allEnabled`, using `projectInterface?.task` in the dependency array may not satisfy hook lint rules and can be fragile if `projectInterface` changes identity; it’s safer to depend on `projectInterface` (or a stable derived value) instead.

## Individual Comments

### Comment 1
<location path="src/stores/appStore.ts" line_range="618-619" />
<code_context>
   // 检查是否有保存的设备和资源配置(用于权限检查等)
   const currentControllerName =
     selectedController[instanceId] || projectInterface?.controller[0]?.name;
+  const currentResourceName =
</code_context>
<issue_to_address>
**suggestion:** The controller/resource resolution logic is duplicated with Toolbar and could be centralized.

This mirrors the resolution logic in `Toolbar` (including fallback order and `projectInterface` usage). Consider extracting a shared helper (e.g., `getCurrentControllerAndResource(state, instanceId)`) so the rules stay consistent and only need updating in one place.

Suggested implementation:

```typescript
export const getCurrentControllerAndResource = (state: AppState, instanceId: string) => {
        const pi = state.projectInterface;
        const instance = state.instances.find((i) => i.id === instanceId);

        const controllerName =
          state.selectedController[instanceId] ||
          instance?.controllerName ||
          pi?.controller[0]?.name;

        const resourceName =
          state.selectedResource[instanceId] ||
          instance?.resourceName ||
          pi?.resource[0]?.name;

        return { controllerName, resourceName };
      };

      set((state) => {

```

```typescript
        const { controllerName: currentControllerName, resourceName: currentResourceName } =
          getCurrentControllerAndResource(state, instanceId);

```

1. In the `Toolbar` component (or wherever the duplicated controller/resource resolution logic currently lives), replace the inline logic with a call to `getCurrentControllerAndResource(state, instanceId)` to keep behavior consistent.
2. Ensure `AppState` (or the correct store state type) is imported or in scope for the helper signature; if the type name differs, adjust `AppState` to the appropriate type used in `appStore.ts`.
3. Export `getCurrentControllerAndResource` from `src/stores/appStore.ts` (if your bundler/TS setup requires explicit exports beyond this declaration) and import it in `Toolbar`.
4. If `Toolbar` does not have access to the raw store state, you may need a small wrapper selector that calls `getCurrentControllerAndResource` via your store (e.g., `useAppStore((state) => getCurrentControllerAndResource(state, instanceId)))`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

修复任务列表“全选”在批量切换时未正确遵循当前控制器/资源兼容性约束的问题,并让工具栏的“全部已选中”状态计算与该约束保持一致。

Changes:

  • selectAllTasks 全选启用时跳过与当前控制器/资源不兼容的任务
  • 工具栏 allEnabled 计算仅统计与当前控制器/资源兼容的任务(使用 useMemo

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/stores/appStore.ts 调整批量启用逻辑,使全选时考虑 controller/resource 兼容性
src/components/Toolbar.tsx 调整“全选已选中”状态计算逻辑以匹配兼容性约束

Copy link
Copy Markdown
Owner

@MistEO MistEO left a comment

Choose a reason for hiding this comment

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

没啥问题,看下 AI 建议有没有要改的

@ShadowLemoon
Copy link
Copy Markdown
Contributor Author

准备提取个工具函数

@MistEO
Copy link
Copy Markdown
Owner

MistEO commented Apr 2, 2026

不过其实全选上也无所谓吧,后面实际执行阶段是会忽略掉不支持的 task 的

@MistEO
Copy link
Copy Markdown
Owner

MistEO commented Apr 2, 2026

57e1896 这条 commit 导致 mxu 启动后画面是白屏,请检查一下

先是
image

然后就一直
image

revert 掉这条 commit 后恢复正常

@ShadowLemoon
Copy link
Copy Markdown
Contributor Author

不过其实全选上也无所谓吧,后面实际执行阶段是会忽略掉不支持的 task 的

end那边实时辅助标着不支持后台,但是确实可以在后台控制器用来确认滑索

@MistEO MistEO merged commit 19af755 into MistEO:main Apr 3, 2026
9 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.

3 participants