[shaohuzhang1]

【【系统管理】- 资源列表、操作日志列表上的工作空间过滤组件不能搜索】
https://www.tapd.cn/tapd_fe/45245355/bug/detail/1145245355001079355

[shaohuzhang1]

Issues Found

1. Semantics of "Select All" under search are ambiguous / possibly buggy

In handleSelectAllChange, when filterable is true, the retained values are computed as:

pendingValues.value.filter((value) => !allOptionValues.value.includes(value))

This works for retaining hidden selections — but there's a subtle issue: when checked is false, this keeps all values not in the filtered set, which is correct. However, the "全部" checkbox itself toggles based on filtered options only. That behavior is fine semantically, but should be documented — users may expect "select all" to mean "all options", not "all visible options". Consider renaming or adding a tooltip.

2. Indeterminate state ignores hidden selections

selectionIndeterminate and allSelected are both computed from allOptionValues (filtered). If a user searches, selects some items, then changes the search, the state of "全部" reflects only the current filter. This is likely intended but can confuse users — make sure it's documented.

3. disabled checkbox in "全部" uses inconsistent logic

:disabled="filterable && !allOptionValues.length"

When filterable is false, this is never disabled — even if there are no options at all. The original code had no disable guard. Consider making it consistent: :disabled="!allOptionValues.length" regardless of filterable.

4. Search input placement renders outside the scrollbar → layout jump

The MkSearchInput is placed above el-scrollbar, so it won't scroll with content. That's usually desirable, but make sure max-height="280px" on the scrollbar doesn't cause the popover to grow oddly. Also, the empty-state (MkEmpty) lives inside the scroll area while the checkbox for "全部" stays visible — verify that looks acceptable when there are 0 filtered results.

5. Missing whitespace-trimmed keyword state reset timing

searchKeyword.value = '' is reset in handleOpen, which fires on @before-enter. This means during the close animation the previous keyword is still shown briefly, which is fine. But if a user reopens quickly, before-enter may not fire. Prefer explicitly resetting in a watcher on visible:

watch(visible, (v) => { if (v) searchKeyword.value = '' })

6. Set spread loses order = OK but could surprise

[...new Set([...retainedValues, ...allOptionValues.value])] preserves insertion order — good. But note that retainedValues comes first, so the resulting order differs from props.options ordering. If consumers depend on order, sort by props.options index instead. Minor.

7. Case sensitivity only lowercases label, not non-string labels

option.label.toLowerCase()

Assumes label is always a string. If TableFilterOption.label can be number/VNode, this will throw. Guard it or type it as string.

8. Performance: filtering + includes inside every/some is O(n·m)

For large option lists this is O(n²) per computed run. Consider using a Set of pendingValues inside those computeds:

const pendingSet = computed(() => new Set(pendingValues.value))

Then use .has(value).

9. Duplicated key generation

key={${typeof option.value}:${option.value}} is repeated in two places — extract to a helper function for readability.

10. No debounce on search

If options is very large, filtering on each keystroke and re-rendering thousands of checkboxes will lag. Consider debouncing searchKeyword (e.g., 150–200ms) or virtualizing the list.

11. Accessibility

• MkSearchInput should have an aria-label (e.g., "Search options").
• MkEmpty inside a role group is fine, but ensure the checkbox group remains correctly labeled after the empty state appears.

---

Summary of recommended fixes (priority order)

1. Guard option.label typing for .toLowerCase().
2. Use a Set for pendingValues lookups in computeds.
3. Reset searchKeyword via a watch on visible rather than before-enter.
4. Unify the disabled logic on the "全部" checkbox.
5. Add debounce to search input.
6. Add aria-label to search.

[shaohuzhang1]

Summary of the change
The only functional change is adding filterable to <MkTableFilter>, which presumably enables a search/filter box inside the dropdown. That's harmless as long as MkTableFilter accepts and correctly handles that prop.

Potential issues / things to verify

1. Prop support & semantics — Confirm MkTableFilter actually declares filterable in its props and implements filtering (e.g., filters options by label/value). If it doesn't, this is a no-op or a silently dropped attribute.

2. Filtering options must be reactive — If workspaceOptions is large or loaded async, make sure the internal filter recomputes when options changes (computed/watch rather than caching).

3. Localization / matching behavior — With Chinese labels ("工作空间"), ensure the filter does case-insensitive / substring matching appropriately; consider whether pinyin or punctuation normalization is expected by users.

4. Large option lists — If workspaceOptions can be hundreds/thousands of entries, filtering on every keystroke may cause jank. Consider debouncing input or using virtual scrolling (el-select-vue / el-select with filterable + multiple supports native filtering but with many items it can lag).

5. Selected values vs filtered results — In multiple-selection mode, filtering hides selected tags less obviously; verify selected items not matching the query are still retained in selectedWorkspaceIds and displayed correctly.

6. @change timing — Filtering is UI-only; handleFilterChange should fire on selection change, not on filter input. Confirm the component doesn't emit change while typing.

7. Reset behavior — After closing/reopening the dropdown, does the filter text reset? Usually desirable; otherwise stale queries confuse users.

8. No-results state — Ensure an empty-state message is shown when the filter matches nothing.

9. Accessibility — Check keyboard navigation still works with the filter input (arrow keys, Esc to clear/close).

Minor style/nit

• Consider keeping the original one-line form if the project's linter/prettier has a low print-width; the multi-line version is fine and arguably more readable.
• Prop order is consistent with existing usage elsewhere — good.
• The diff hunk header @@ -156,7 +156,14 @@ indicates 7 lines replaced by 14 (one line split into a block); looks correct given the change.

Overall: No irregularities in syntax. The main risk is whether filterable is supported and behaves well with large/asynchronous option sets — worth a quick test with many workspaces and verifying selection persistence during filtering.

[shaohuzhang1]

The change itself is minor (just adding a filterable prop and reformatting), but here are some observations:

Potential issues

1. Missing key on the column – Not critical since it's static, but if columns toggle via v-if, ensure reactivity is fine.

2. filterable without client-side fallback – Verify that MkTableFilter handles large workspaceOptions lists well when filterable is enabled. If the list is huge, consider server-side search instead of local filtering.

3. v-model + @change duplication – You're using both two-way binding (v-model="selectedWorkspaceIds") and @change="handleFilterChange". Make sure handleFilterChange doesn't rely on stale values; with v-model the update order matters (the change event may fire before the model updates in some implementations).

4. auth.isEE gating – Good to confirm that workspaceOptions is populated before this component renders, otherwise filterable will just show an empty dropdown.

Optimization suggestions

1. Consistent option labeling – For filterable UX, make sure each option has a clear, unique label string so the user can match by typing.

2. Add debounce for filtering – If MkTableFilter filters locally and the options array is large (>200), debounce the input or use virtual scrolling.

3. Preserve filter state – When switching pages or re-mounting (e.g., route change), consider persisting selectedWorkspaceIds if this is expected behavior.

4. Extract repeated props – If filterable becomes common across multiple MkTableFilter usages, consider defaulting it inside MkTableFilter rather than repeating it.

Minor style

• The multi-line formatting is fine for readability, though for just one added attribute you could keep it inline.
• No functional irregularities detected in this hunk — the diff is low-risk.