Skip to content

refactor: Password input constractor #2430

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
liuruibin merged 1 commit into from
Feb 27, 2025
Merged

refactor: Password input constractor #2430

Show file tree
Hide file tree
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: 4 additions & 0 deletions ui/src/components/dynamics-form/constructor/data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ const input_type_list = [
label: t('dynamicsForm.input_type_list.TextInput'),
value: 'TextInput'
},
{
label: t('dynamicsForm.input_type_list.PasswordInput'),
value: 'PasswordInput'
},
{
label: t('dynamicsForm.input_type_list.Slider'),
value: 'Slider'
Expand Down
194 changes: 194 additions & 0 deletions ui/src/components/dynamics-form/constructor/items/PasswordInputConstructor.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
<template>
<el-form-item :label="$t('dynamicsForm.TextInput.length.label')" required>
<el-row class="w-full">
<el-col :span="11">
<el-form-item
:rules="[
{
required: true,
message: $t('dynamicsForm.TextInput.length.minRequired'),
trigger: 'change'
}
]"
prop="minlength"
>
<el-input-number
style="width: 100%"
:min="1"
:step="1"
step-strictly
v-model="formValue.minlength"
controls-position="right"
/>
</el-form-item>
</el-col>
<el-col :span="2" class="text-center">
<span>-</span>
</el-col>
<el-col :span="11">
<el-form-item
:rules="[
{
required: true,
message: $t('dynamicsForm.TextInput.length.maxRequired'),
trigger: 'change'
}
]"
prop="maxlength"
>
<el-input-number
style="width: 100%"
:min="formValue.minlength > formValue.maxlength ? formValue.minlength : 1"
step-strictly
:step="1"
v-model="formValue.maxlength"
controls-position="right"
/>
</el-form-item>
</el-col>
</el-row>
</el-form-item>

<el-form-item
class="defaultValueItem"
:required="formValue.required"
prop="default_value"
:label="$t('dynamicsForm.default.label')"
:rules="
formValue.required ? [{ required: true, message: `${$t('dynamicsForm.default.label')}${$t('dynamicsForm.default.requiredMessage')}` }, ...rules] : rules
"
>
<div class="defaultValueCheckbox">
<el-checkbox
v-model="formValue.show_default_value"
:label="$t('dynamicsForm.default.show')"
/>
</div>

<el-input
v-model="formValue.default_value"
:maxlength="formValue.maxlength"
:minlength="formValue.minlength"
:placeholder="$t('dynamicsForm.default.placeholder')"
show-word-limit
type="password"
show-password
/>
</el-form-item>
</template>
<script setup lang="ts">
import { computed, onMounted, watch } from 'vue'
import { t } from '@/locales'

const props = defineProps<{
modelValue: any
}>()
const emit = defineEmits(['update:modelValue'])
const formValue = computed({
set: (item) => {
emit('update:modelValue', item)
},
get: () => {
return props.modelValue
}
})
watch(
() => formValue.value.minlength,
() => {
if (formValue.value.minlength > formValue.value.maxlength) {
formValue.value.maxlength = formValue.value.minlength
}
}
)
const getData = () => {
return {
input_type: 'PasswordInput',
attrs: {
maxlength: formValue.value.maxlength,
minlength: formValue.value.minlength,
'show-word-limit': true,
type: 'password',
'show-password': true
},
default_value: formValue.value.default_value,
show_default_value: formValue.value.show_default_value,
props_info: {
rules: formValue.value.required
? [
{
required: true,
message: `${formValue.value.label} ${t('dynamicsForm.default.requiredMessage')}`
},
{
min: formValue.value.minlength,
max: formValue.value.maxlength,
message: `${formValue.value.label}${t('dynamicsForm.TextInput.length.requiredMessage1')} ${formValue.value.minlength} ${t('dynamicsForm.TextInput.length.requiredMessage2')} ${formValue.value.maxlength} ${t('dynamicsForm.TextInput.length.requiredMessage3')}`,
trigger: 'blur'
}
]
: [
{
min: formValue.value.minlength,
max: formValue.value.maxlength,
message: `${formValue.value.label}${t('dynamicsForm.TextInput.length.requiredMessage1')} ${formValue.value.minlength} ${t('dynamicsForm.TextInput.length.requiredMessage2')} ${formValue.value.maxlength} ${t('dynamicsForm.TextInput.length.requiredMessage3')}`,
trigger: 'blur'
}
]
}
}
}
const rander = (form_data: any) => {
const attrs = form_data.attrs || {}
formValue.value.minlength = attrs.minlength
formValue.value.maxlength = attrs.maxlength
formValue.value.default_value = form_data.default_value
formValue.value.show_default_value = form_data.show_default_value
formValue.value.show_password = attrs['show-password']
}
const rangeRules = [
{
required: true,
validator: (rule: any, value: any, callback: any) => {
if (!formValue.value.minlength) {
callback(new Error(t('dynamicsForm.TextInput.length.requiredMessage4')))
}
if (!formValue.value.maxlength) {
callback(new Error(t('dynamicsForm.TextInput.length.requiredMessage4')))
}
return true
},
message: `${formValue.value.label} ${t('dynamicsForm.default.requiredMessage')}`
}
]
const rules = computed(() => [
{
min: formValue.value.minlength,
max: formValue.value.maxlength,
message: `${t('dynamicsForm.TextInput.length.requiredMessage1')} ${formValue.value.minlength} ${t('dynamicsForm.TextInput.length.requiredMessage2')} ${formValue.value.maxlength} ${t('dynamicsForm.TextInput.length.requiredMessage3')}`,
trigger: 'blur'
}
])

defineExpose({ getData, rander })
onMounted(() => {
formValue.value.minlength = 0
formValue.value.maxlength = 20
formValue.value.default_value = ''
formValue.value.show_password = true

if (formValue.value.show_default_value === undefined) {
formValue.value.show_default_value = true
}
})
</script>
<style lang="scss" scoped>
.defaultValueItem {
position: relative;

.defaultValueCheckbox {
position: absolute;
right: 0;
top: -35px;
}
}
</style>
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Feb 27, 2025

Choose a reason for hiding this comment

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

The code looks mostly well-formed, but there are a few areas that could be optimized or corrected:

  1. HTML Structure: The HTML structure is correct and semantically appropriate.

  2. TypeScript Setup: The setup function in Vue 3 uses TypeScript correctly with defineProperties, defineEmits, computed, watch, etc.

  3. Computed Properties and Watches: The computed properties (formValue) and watches are functioning as expected to handle updates.

  4. Data Methods:

    • getData: This method correctly returns an object containing the form data.
    • rander: This method updates the form values based on incoming form data.

  5. Validation Rules:

    • The validation rules in rangeRules ensure that both fields have valid lengths before submission.
    • The rules computation checks if the length constraints are met when needed.

  6. Style Scopes: The SASS scope is properly defined within .defaultValueItem.

Suggestions for Improvement:

  • Default Values in Template: Currently, the placeholder text for default_value is hardcoded. Make sure this should come from translations if available.

  • Password Visibility Toggle: Ensure the password visibility toggle functionality works as intended. If it doesn't, consider using a library like vue-password-toggle.

  • Testing Edge Cases: Although not critical here, thoroughly test extreme cases such as very short and long string inputs to ensure all conditions behave as expected.

Additional Considerations:

  • Accessibility: Ensure the page meets accessibility standards. For example, make sure aria labels are used appropriately.

  • Internationalization: Since t comes from '@/locales', ensure $route.name (or similar dynamic route handling) is working correctly to provide context-sensitive translation messages.

Overall, the existing code is functional and follows best practices. Minor improvements can enhance readability and maintainability.

18 changes: 3 additions & 15 deletions ui/src/components/dynamics-form/constructor/items/TextInputConstructor.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
<template>
<el-form-item
class="defaultValueItem"
prop="show_password"
:label="$t('dynamicsForm.TextInput.showPassword')"
>
<el-switch v-model="formValue.show_password" />
</el-form-item>
<el-form-item :label="$t('dynamicsForm.TextInput.length.label')" required>
<el-row class="w-full">
<el-col :span="11">
Expand Down Expand Up @@ -77,8 +70,7 @@
:minlength="formValue.minlength"
:placeholder="$t('dynamicsForm.default.placeholder')"
show-word-limit
:type="formValue.show_password ? 'password' : 'text'"
:show-password="formValue.show_password"
type="text"
/>
</el-form-item>
</template>
Expand Down Expand Up @@ -111,9 +103,7 @@ const getData = () => {
attrs: {
maxlength: formValue.value.maxlength,
minlength: formValue.value.minlength,
'show-word-limit': true,
type: formValue.value.show_password ? 'password' : 'text',
'show-password': formValue.value.show_password
'show-word-limit': true
},
default_value: formValue.value.default_value,
show_default_value: formValue.value.show_default_value,
Expand Down Expand Up @@ -145,7 +135,6 @@ const rander = (form_data: any) => {
formValue.value.maxlength = attrs.maxlength
formValue.value.default_value = form_data.default_value
formValue.value.show_default_value = form_data.show_default_value
formValue.value.show_password = attrs['show-password']
}
const rangeRules = [
{
Expand Down Expand Up @@ -176,8 +165,7 @@ onMounted(() => {
formValue.value.minlength = 0
formValue.value.maxlength = 20
formValue.value.default_value = ''
formValue.value.show_password = false

// console.log(formValue.value.show_default_value)
if (formValue.value.show_default_value === undefined) {
formValue.value.show_default_value = true
}
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Feb 27, 2025

Choose a reason for hiding this comment

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

There are several issues and improvements that can be made to this Vue.js template and JavaScript code:

  1. Type Script Mismatch: The maxlength and minlength attributes should use numbers instead of strings. This is consistent with how input fields expect numerical values.

  2. Switch Label Issue: Since you have an <el-switch> element for "Show Password," ensure the label matches the switch state correctly. Currently, it seems redundant since the value comes from formValue.show_password.

  3. Remove Redundant Attributes: You shouldn't set both type and show-password directly because they might conflict or cause unexpected behavior. Remove one of them.

  4. Avoid Global State Updates: Directly updating global variables like formValue.value in getData, rander, and other functions can lead to inconsistencies if not handled properly. Instead, consider using computed properties, reactive refs, or Vuex for a more clean and maintainable approach.

  5. Comments and Logging: While logging (console.log(formValue.value.show_default_value)) has its purpose, try avoiding unnecessary logging unless debugging is needed.

Here's a revised version of the code:

<template>
  <el-form-item :label="$t('dynamicsForm.TextInput.length.label')" required>
    <el-row class="w-full">
      <el-col :span="11">
        <el-input-number v-model.number="value.maxlength" placeholder="Enter max length" />
      </el-col>
      <el-col :span="11">
        <el-input-number v-model.number="value.minlength" placeholder="Enter min length" />
      </el-col>
    </el-row>

    <el-form-item :label="$t('dynamicsForm.TextInput.text.type')">Text Type</el-form-item>
    <el-radio-group v-model:value.type>
        <el-radio-button label="text">Text Input</el-radio-button>
        <el-radio-button label="password">Password Input</el-radio-button>
    </el-radio-group>

    <el-form-item :label="$t('dynamicsForm.TextInput.placeholder')">Placeholder</el-form-item>
    <el-input v-model:value.placeholder />

    <el-form-item :label="$t('dynamicsForm.TextInput.default')">
      Default Value <em>(Optional)</em></el-form-item>
    <el-input v-model:value.default-value />
  </el-form-item>
</template>

<script lang="ts">
import { defineComponent } from 'vue';
// Assume formValue is defined somewhere and imported
export default defineComponent({
  data() {
    return {
      value: {
        maxlen: null,
        minlength: null,
        type: 'text',
        placeholder: '',
        default_value: '',
        show_default_value: true,
      },
    };
  },
});
</script>

In this improved version:

  • The v-model.number directive ensures numeric handling for maxlength and minlength.
  • Removed redundancy by setting just one attribute (type) based on radio button selection.
  • Used a structured approach for managing form values and types.
  • Simplified logic through the introduction of a value object encapsulating all field-related properties.

Expand Down
4 changes: 2 additions & 2 deletions ui/src/locales/lang/en-US/dynamics-form.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export default {
input_type_list: {
TextInput: 'Input',
PasswordInput: 'Password',
Slider: 'Slider',
SwitchInput: 'Switch',
SingleSelect: 'Single Select',
Expand Down Expand Up @@ -96,7 +97,6 @@ export default {
requiredMessage2: 'and',
requiredMessage3: 'characters',
requiredMessage4: 'Text length is a required parameter'
},
showPassword: 'Show Password'
}
}
}
4 changes: 2 additions & 2 deletions ui/src/locales/lang/zh-CN/dynamics-form.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export default {
input_type_list: {
TextInput: '文本框',
PasswordInput: '密码框',
Slider: '滑块',
SwitchInput: '开关',
SingleSelect: '单选框',
Expand Down Expand Up @@ -96,7 +97,6 @@ export default {
requiredMessage2: '到',
requiredMessage3: '个字符',
requiredMessage4: '文本长度为必填参数'
},
showPassword: '密文显示'
}
}
}
4 changes: 2 additions & 2 deletions ui/src/locales/lang/zh-Hant/dynamics-form.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export default {
input_type_list: {
TextInput: '文字框',
PasswordInput: '密文框',
Slider: '滑桿',
SwitchInput: '開關',
SingleSelect: '單選框',
Expand Down Expand Up @@ -96,7 +97,6 @@ export default {
requiredMessage2: '到',
requiredMessage3: '個字元',
requiredMessage4: '文字長度為必填參數'
},
showPassword: '密文顯示'
}
}
}
1 change: 1 addition & 0 deletions ui/src/workflow/nodes/base-node/component/UserFieldFormDialog.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ const currentRow = computed(() => {
const currentIndex = ref(null)
const inputTypeList = ref([
{ label: t('dynamicsForm.input_type_list.TextInput'), value: 'TextInputConstructor' },
{ label: t('dynamicsForm.input_type_list.PasswordInput'), value: 'PasswordInputConstructor' },
{ label: t('dynamicsForm.input_type_list.SingleSelect'), value: 'SingleSelectConstructor' },
{ label: t('dynamicsForm.input_type_list.MultiSelect'), value: 'MultiSelectConstructor' },
{ label: t('dynamicsForm.input_type_list.RadioCard'), value: 'RadioCardConstructor' },
Expand Down
6 changes: 6 additions & 0 deletions ui/src/workflow/nodes/base-node/component/UserInputFieldTable.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
<el-tag type="info" class="info-tag" v-if="row.input_type === 'TextInput'">{{
$t('dynamicsForm.input_type_list.TextInput')
}}</el-tag>
<el-tag type="info" class="info-tag" v-if="row.input_type === 'PasswordInput'">{{
$t('dynamicsForm.input_type_list.PasswordInput')
}}</el-tag>
<el-tag type="info" class="info-tag" v-if="row.input_type === 'Slider'">{{
$t('dynamicsForm.input_type_list.Slider')
}}</el-tag>
Expand Down Expand Up @@ -169,6 +172,9 @@ function refreshFieldTitle(data: any) {
}

const getDefaultValue = (row: any) => {
if(row.input_type === 'PasswordInput') {
return '******'
}
if (row.default_value) {
const default_value = row.option_list
?.filter((v: any) => row.default_value.indexOf(v.value) > -1)
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Feb 27, 2025

Choose a reason for hiding this comment

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

There are no immediate issues with the provided code snippet. It appears to be correct in terms of syntax and structure, though there's an empty else block after the first if statement in the getDefaultValue function that could potentially cause runtime errors if not used correctly.

Here is the updated version with potential improvements suggested:

// Function to check input types and display corresponding tags
function handleInputTypeTags(row) {
  switch (row.input_type) {
    case 'TextInput':
      return <el-tag type="info" class="info-tag">{{ $t('dynamicsForm.input_type_list.TextInput') }}</el-tag>;
    case 'PasswordInput':
      return (
        <>
          <el-tag type="info" class="info-tag">
            {{ $t('dynamicsForm.input_type_list.PasswordInput') }}
          </el-tag>
          {/* Show the password field placeholder here */}
        </>
      );
    case 'Slider':
      return <el-tag type="info" class="info-tag">{{ $t('dynamicsForm.input_type_list.Slider') }}</el-tag>;
    default:
      // Handle other cases if needed
      break;
  }
}

// Update the refreshFieldTitle function usage
const refreshFieldTitle = (data) => {
  // Use a loop to process each element in data array using handleInputTypeTags
};

// Updated getDefaultValue function handling PasswordInput
const getDefaultValue = (row) => {
  if (row.default_value && row.option_list) {
    const filteredValues = row.option_list.filter((v) =>
      row.default_value.includes(v.value)
    );
    if (filteredValues.length > 0) {
      return filteredValues[0].value; // Return the first available value from option_list
    } else {
      return 'No matching values found';
    }
  }
};

Optimization Suggestions:

  1. Switch Statement for Input Type Handling: The use of a switch statement makes the logic more readable and easier to maintain compared to multiple if/else if statements.

  2. Placeholder Display in PasswordTag: In the modified handleInputTypeTags, added a placeholder to indicate where you can add the actual password field when displaying this tag. This prevents accidental showing of passwords in plain text.

  3. Handling Case Where No Matching Values Found: Added a message indicating what should happen if no matching values are found. Adjust based on your application needs.

  4. Default Value Filtering: Enhanced getDefaultValues method by returning only one value if multiple matches exist, ensuring it behaves consistently across different scenarios.

These changes make the code cleaner and more functional while keeping performance considerations in mind where necessary.

Expand Down