Skip to content

fix: workflow knowledge quote variable text #7753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fit2cloudwxx merged 1 commit into from
Nov 9, 2026
Merged

fix: workflow knowledge quote variable text #7753

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ui/src/workflow-canvas/component/node-search-scope/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,12 @@ onBeforeUnmount(() => anchorGuard.reset())
:validate-event="false"
:teleported="false"
size="small"
class="w-18!"
class="w-22!"
@wheel="handleNodeWheel"
@update:model-value="changeScopeType"
@visible-change="anchorGuard.setOverlayVisible('scope-type', $event)"
>
<el-option label="引用" value="referencing" />
<el-option label="引用变量" value="referencing" />
<el-option label="自定义" value="custom" />
</el-select>
</div>

@shaohuzhang1 shaohuzhang1 Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Summary of the diff

Two changes in a Vue 3 + Element Plus <el-select>:

  1. class="w-18!" → class="w-22!" (wider select, presumably to fit the longer label)
  2. <el-option label="引用" …> → <el-option label="引用变量" …> (option value unchanged — good, no data migration needed)

Both are functionally safe. The main risks are around whether w-22 actually resolves, whether 88px is wide enough, and tests / other places that hard-code the old label.

Issues & risks

1. w-22 may silently do nothing (highest risk)

The ! suffix means Tailwind v4 (v3 uses the prefix !w-18), where the spacing scale is dynamic, so w-22 = 5.5rem = 88px and works. But if this project actually uses UnoCSS / preset-wind, w-22 is not in the default spacing scale — the class won't be generated, no build error occurs, and the select silently falls back to its intrinsic width. That's a layout regression that's easy to miss.

Suggestion: verify the class is emitted, or use an explicit value that can't be a no-op:

class="w-[88px]!"      <!-- or min-w-[88px]! -->

2. 88px is tight for "引用变量"

size="small" leaves roughly ~20–28px for the arrow icon and ~16px of horizontal padding, so you have ~45–50px of text room. Four CJK glyphs at 14px ≈ 56px → text will be ellipsized/truncated when "引用变量" is the selected value. The dropdown list is fine (it sizes to content), but the closed control is not.

Suggestion: check the rendered width, and consider min-w instead of a hard w, e.g. min-w-22!, or let the control size to content. Also verify the popper isn't clipped given :teleported="false".

3. Hard-coded label vs. i18n

引用变量 (and the sibling 自定义) are raw Chinese strings. That's at least consistent, but if the project has i18n, these should be locale keys, and the label now has two different terminologies in the codebase ("引用" elsewhere vs "引用变量" here). Centralize them:

const SCOPE_TYPE_LABELS = {
  referencing: '引用变量',
  custom: '自定义',
} as const

Drift between this label and any other place that renders the scope type (tooltips, filter chips, table columns, log messages, aria-labels) is the most likely follow-up bug.

4. Tests / snapshots / e2e selectors will break

Search the repo for the literal 引用:

  • unit tests asserting option text or wrapper.find('…').text()
  • e2e getByText('引用') — note a non-exact matcher now also matches "引用变量", so some tests may start matching the wrong element rather than fail loudly
  • i18n snapshot files

Suggestion: prefer data-test attributes over text selectors so label changes don't break tests.

5. The !important utility is a smell

w-18! / w-22! indicates the width is fighting Element Plus's own CSS. If this is a recurring pattern, it's more maintainable to set it once via a scoped class or CSS variable:

<el-select class="scope-type-select" …>
.scope-type-select { width: 5.5rem; }
/* or */ .scope-type-select { --el-select-width: 5.5rem; }

Also, 22 is a magic number — if there's a design-token scale, use it.

Other observations in the surrounding (unchanged) code

  • @wheel="handleNodeWheel" on an el-select — the most common bug here is scrolling the page over a focused select and accidentally changing the value. Make sure the handler calls preventDefault() and/or blurs, otherwise users will silently switch scope types while scrolling. If the intent is only "let the page scroll", use @wheel.prevent.stop.
  • :teleported="false" — required inside transformed/stacked containers (e.g. dialogs), but it means the popper is clipped by overflow: hidden ancestors and participates in the local z-index stack. Verify it renders above the surrounding panel; otherwise keep it teleported and use :popper-class for positioning.
  • :validate-event="false" — only disables triggering form validation on change. Fine if this field isn't in an el-form, but if it is, this value will never be validated on user interaction.
  • onBeforeUnmount(() => anchorGuard.reset()) vs @visible-change — if the component unmounts with the dropdown open, visible-change(false) can fire after reset(), re-dirtying the guard state. Cheap fix: guard the handler with an isUnmounted flag, or reset inside onUnmounted instead of onBeforeUnmount.
  • @visible-change="anchorGuard.setOverlayVisible('scope-type', $event)" — implicit $event works, but an explicit arrow (visible: boolean) => anchorGuard.setOverlayVisible('scope-type', visible) is clearer and type-checked, and avoids the method being treated as a listener that receives extra args.
  • @update:model-value="changeScopeType" — Element Plus emits string | number | boolean | Record | Array. If changeScopeType expects a narrow union, tighten the signature or add a runtime check so an unexpected value doesn't silently fall through the referencing | custom branches.

Suggested version of the changed block

<el-select
  v-model="scopeType"
  :validate-event="false"
  :teleported="false"
  size="small"
  class="scope-type-select"   <!-- width via CSS, no !important -->
  :aria-label="t('scopeType.label')"
  @wheel="handleNodeWheel"
  @update:model-value="changeScopeType"
  @visible-change="(visible) => anchorGuard.setOverlayVisible('scope-type', visible)"
>
  <el-option :label="t('scopeType.referencing')" value="referencing" />  <!-- 引用变量 -->
  <el-option :label="t('scopeType.custom')" value="custom" />
</el-select>
.scope-type-select { min-width: 5.5rem; } /* grows instead of truncating */

Net: the change itself is low-risk and the value was correctly left untouched. Before merging, confirm (a) w-22 is actually generated by the CSS engine in use, (b) "引用变量" isn't truncated at 88px, and (c) no test or other label map still references the old "引用" text.

Expand Down
Loading