Skip to content

feat: Support single line multi select cards#3976

Merged
shaohuzhang1 merged 1 commit intov2from
pr@v2@feat_multi_row
Sep 1, 2025
Merged

feat: Support single line multi select cards#3976
shaohuzhang1 merged 1 commit intov2from
pr@v2@feat_multi_row

Conversation

@shaohuzhang1
Copy link
Copy Markdown
Contributor

feat: Support single line multi select cards

@f2c-ci-robot
Copy link
Copy Markdown

f2c-ci-robot bot commented Sep 1, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@f2c-ci-robot
Copy link
Copy Markdown

f2c-ci-robot bot commented Sep 1, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

:deep(.el-select-dropdown) {
max-width: 400px;
}
</style>
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.

The provided code for a Vue.js component to render a dynamic form is mostly well-written. Here are some considerations for improvement and potential optimizations:

  1. Code Duplicacy: There's significant duplicated logic in handling the assignment_method selection, especially when setting up default_ref_variables_value_rule. Consider abstracting this into reusable functions.

  2. Error Handling: The function that validates the ref_variables values does not handle cases where the first item of the validation array is incorrect. Consider adding early returns or more robust error messages.

  3. Performance Optimization: The use of deep selectors like :deep() can be overkill, particularly if other components might also use these styles in unintended ways. It would be better to isolate specific classes within their respective components.

  4. Consistent Formatting: Ensure consistent spacing and line breaks within comments and strings for easier readability.

  5. Event Emission: Although not explicitly shown, ensure there’s no missing events being emitted that could affect behavior elsewhere in the application.

Here is a revised version with suggested improvements:

<script setup lang="ts">
import { computed, onMounted, inject } from 'vue';
import MultiRow from '@/components/dynamics-form/items/MultiRow.vue';
import NodeCascader from '@/workflow/common/NodeCascader.vue';
import type { FormField } from '@/components/dynamics-form/type';

const getModel = inject('getModel') as any;

// Abstracted function for validating default_ref_variables_value_rule
function validateRefVariables(rule: any, value: any): boolean {
  if (!Array.isArray(value) || value.length <= 1) {
    return false;
  }
  // Additional custom validations if needed
  return true;
}

const assignment_method_option_list = computed(() => {
  const option_list = [
    {
      label: t('dynamicsForm.AssignmentMethod.custom.label', '自定义'),
      value: 'custom',
    },
  ];

  if (getModel) {
    option_list.push({
      label: t('dynamicsForm.AssignmentMethod.ref_variables.label', '引用变量'),
      value: 'ref_variables',
    });
  }

  return option_list;
});

... rest of the script remains the same

</script>

<style scoped>
.defaultValueItem {
  position: relative;
  .defaultValueCheckbox {
    position: absolute;
    right: 0;
    top: -35px;
  }
}

/* Isolated deeper selector */
/deep/.el-select-dropdown {
  max-width: 400px;
}
</style> 

These adjustments improve the maintainability and consistency of the code while ensuring functionality remains intact.

}
}
}
</style>
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.

The code looks mostly clean, but here are a few recommendations:

  1. Avoid void 0 injection: Instead of injecting into formItemContextKey, directly use the injected function to access the validation method.

  2. Use concise template expression syntax: This makes the code more readable and maintainable.

  3. Refactor conditional classes: Use CSS variables instead of hardcoded colors for better flexibility.

Here's how you could refactor it:

<template>
  <div class="radio_content">
    <div
      v-for="item in option_list"
      :key="item[value_field]"
      class="item"
      :class="{ is_disabled: !inputDisabled, [activeClass]: isActive(item) }"
      @click="selected(item[value_field])"
    >
      {{ item[textField] }}
    </div>
  </div>
</template>

<script lang="ts" setup>
import { computed, inject } from 'vue';
import type { FormField } from '@/components/dynamics-form/type';
import formItemContextKey from 'element-plus/es/components/form/src/constants';

const inputDisabled = inject(formItemContextKey);

defineProps<{
  formValue?: any;
  formffieldList?: Array<FormField>;
  field: string;
  otherParams: any;
  formField: FormField;
  view?: boolean;
  modelValue?: any;
}>();

const elFormItem = inject(formItemContextKey) as unknown as ElFormComponent;

function selected(activeValue: string | number) {
  const newValue = props.modelValue || [];
  
  if (!newValue.includes(activeValue)) {
    newValue.push(activeValue);
  } else {
    const index = newValue.findIndex(i => i === activeValue);
    newValue.splice(index, 1);
  }

  emit('update:modelValue', newValue);
  if (elFormItem && elFormItem.validate) {
    elFormItem.clearValidate();
  }
}

const textField = computed(() => props.formField.text_field || 'key');
const valueField = computed(() => props.formField.value_field || 'value');

const option_list = computed(() => props.formField.option_list || []);

function isActive(item: { [key: string]: string }) {
  return isActiveComputed().includes(item[value_field]);
}

// Calculate the active state based on modelValue
const isActiveComputed = computed(() => props.modelValue || []);
</script>

<style lang="scss" scoped>
.radio_content {
  $border-color: #bbbfc4;
  $primary-bg-light-9: lighten(#57aaff, 5%);
  $disabled-bg: lighten(#ccc, 6%);

  height: 32px;
  display: inline-flex;
  border: 1px solid $border-color;
  border-radius: 4px;
  font-weight: 400;
  font-size: 14px;
  color: var(--el-text-color-primary);
  padding: 3px 4px;
  box-sizing: border-box;
  white-space: nowrap;

  &.is-disabled {
    border: 1px solid $disable-bg;
    background: $disabled-bg;
    color: var(--el-text-color-placeholder);
    cursor: not-allowed;
    &:hover {
      cursor: not-allowed;
    }
  }

  .active {
    border-radius: 4px;
    background: $primary-bg-light-9;
    color: #57aaff;
  }

  .item {
    cursor: pointer;
    margin: 0 2px;
    padding: 2px 8px;
    height: 20px;
    display: flex;
    justify-content: center;
    align-items: center;
    &:last-child {
      margin: 0 4px 0 2px;
    }
    &:first-child {
      margin: 0 2px 0 4px;
    }
  }
}
</style>

This refactoring improves readability and simplifies some logic while using modern Vue.js best practices.

if ['SingleSelect', 'MultiSelect', 'RadioCard', 'RadioRow', 'MultiRow'].__contains__(field.get('input_type')):
if field.get('assignment_method') == 'ref_variables':
option_list = self.workflow_manage.get_reference_field(field.get('option_list')[0],
field.get('option_list')[1:])
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.

There are no significant irregularities or potential issues in this code. However, there are a few minor optimizations that can be made:

  1. Replace list.__contains__ with in operator in both if statements to improve readability.

  2. Move the declaration of multi_select_list outside the functions and define it at class level, where applicable, especially if used frequently across multiple methods. This will make the function more concise and reusable.

  3. Consider using a set instead of list for multi_select_list since sets offer O(1) average time complexity for membership checks.

Here's an example of how you could refactor these changes:

# Define multi-select options as a set for faster membership-checking
multi_select_set = {'.MultiSelect', 'MultiRow'}

def get_default_option(option_list, _type, value_field):
    # ... rest of the code remains similar ...

def reset_field(self, field):
    # ... rest of the code remains similar ...

These small improvements enhance readability and maintainability while keeping the logic essentially unchanged.

@shaohuzhang1 shaohuzhang1 merged commit 67bb0a0 into v2 Sep 1, 2025
3 of 6 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@v2@feat_multi_row branch September 1, 2025 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant