Dataset Creation UI fixes & Improvements (#5670)

- [x] Rating values validate min / max - [x] Question name format and renaming - [x] Minor style adjustments - [x] Show required / not required in the form preview - [x] Redirect from /new/repoId to /repoId - [x] Redirect to error page when repoId is invalid --------- Co-authored-by: Francisco Aranda <[email protected]> Co-authored-by: Paco Aranda <[email protected]> Co-authored-by: José Francisco Calvo <[email protected]> Co-authored-by: José Francisco Calvo <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
argilla-io · Nov 8, 2024 · 6199bb8 · 6199bb8
1 parent 69df2bc
commit 6199bb8
Show file tree

Hide file tree

Showing 15 changed files with 181 additions and 66 deletions.
diff --git a/argilla-frontend/CHANGELOG.md b/argilla-frontend/CHANGELOG.md
@@ -19,11 +19,14 @@ These are the section headers that we use:
 ### Added
 
 - Add a high-contrast theme & improvements for the forced-colors mode. ([#5661](https://github.com/argilla-io/argilla/pull/5661))
+- Added redirect to error page when repoId is invalid ([#5670](https://github.com/argilla-io/argilla/pull/5670))
 
 ### Fixed
 
 - Fixed redirection problems after users sign-in using HF OAuth. ([#5635](https://github.com/argilla-io/argilla/pull/5635))
 - Fixed highlighting of the searched text in text and chat fields ([#5678](https://github.com/argilla-io/argilla/pull/5678))
+- Fixed validation for rating question when creating a dataset ([#5670](https://github.com/argilla-io/argilla/pull/5670))
+- Fixed question name based on question type when creating a dataset ([#5670](https://github.com/argilla-io/argilla/pull/5670))
 
 ## [2.4.0](https://github.com/argilla-io/argilla/compare/v2.3.0...v2.4.0)
 

diff --git a/...-frontend/components/features/dataset-creation/configuration/DatasetConfigurationForm.vue b/...-frontend/components/features/dataset-creation/configuration/DatasetConfigurationForm.vue
@@ -31,13 +31,11 @@
               :group="{ name: 'fields' }"
               ghost-class="config-form__ghost"
               :disabled="isFocused"
-              @start="drag = true"
-              @end="drag = false"
             >
               <transition-group
                 class="config-form__draggable-area-wrapper"
                 type="transition"
-                :name="!drag ? 'flip-list' : null"
+                :css="false"
               >
                 <DatasetConfigurationField
                   v-for="field in dataset.selectedSubset.fields.filter(
@@ -87,7 +85,7 @@
               <transition-group
                 class="config-form__draggable-area-wrapper"
                 type="transition"
-                :name="!drag ? 'flip-list' : null"
+                :css="false"
               >
                 <DatasetConfigurationQuestion
                   v-for="question in dataset.selectedSubset.questions"
@@ -139,22 +137,50 @@ export default {
     return {
       isFocused: false,
       visibleDatasetCreationDialog: false,
-      drag: false,
     };
   },
+  computed: {
+    getMaxNumberInNames() {
+      return Math.max(
+        ...this.dataset.selectedSubset.questions.map((question) => {
+          const numberInName = question.name.split("_").pop();
+          return parseInt(numberInName) || 0;
+        })
+      );
+    },
+  },
   methods: {
     createDataset() {
       this.create(this.dataset);
     },
-    addQuestion(type) {
-      const questionName = `${type} ${this.dataset.selectedSubset.questions.length}`;
-      this.dataset.selectedSubset.addQuestion(questionName, { type });
+    generateName(type, number) {
+      const typeName = this.$t(`config.questionId.${type}`);
+      return `${typeName}_${parseInt(number) || 0}`;
     },
-    onTypeIsChanged(name, type) {
-      this.dataset.selectedSubset.addQuestion(name, {
-        type: type.value,
+    addQuestion(type) {
+      const questionName = this.generateName(
+        type,
+        this.getMaxNumberInNames + 1
+      );
+      this.dataset.selectedSubset.addQuestion(questionName, {
+        type,
       });
     },
+    onTypeIsChanged(oldName, type) {
+      const numberInName = oldName.split("_").pop();
+      const index = this.dataset.selectedSubset.questions.findIndex(
+        (q) => q.name === oldName
+      );
+      this.dataset.selectedSubset.removeQuestion(oldName);
+      const newQuestionName = this.generateName(type.value, numberInName);
+      this.dataset.selectedSubset.addQuestion(
+        newQuestionName,
+        {
+          type: type.value,
+        },
+        index !== -1 ? index : undefined
+      );
+    },
   },
   setup() {
     return useDatasetConfigurationForm();
@@ -223,7 +249,6 @@ export default {
   }
   &__ghost {
     opacity: 0.5;
-    background: lime;
   }
   &__selector {
     &__intro {
@@ -247,8 +272,5 @@ export default {
       justify-content: center;
     }
   }
-  .flip-list-move {
-    transition: transform 0.3s;
-  }
 }
 </style>
diff --git a/...onents/features/dataset-creation/configuration/questions/DatasetConfigurationQuestion.vue b/...onents/features/dataset-creation/configuration/questions/DatasetConfigurationQuestion.vue
@@ -29,7 +29,7 @@
       />
       <DatasetConfigurationRating
         v-else-if="question.settings.type.isRatingType"
-        v-model="question.settings.options"
+        :question="question"
         @is-focused="$emit('is-focused', $event)"
       />
       <DatasetConfigurationRanking

diff --git a/...ponents/features/dataset-creation/configuration/questions/DatasetConfigurationRanking.vue b/...ponents/features/dataset-creation/configuration/questions/DatasetConfigurationRanking.vue
@@ -98,6 +98,7 @@ export default {
     width: 100%;
     outline: none;
     color: var(--fg-secondary);
+    @include font-size(12px);
     @include input-placeholder {
       color: var(--fg-tertiary);
     }

diff --git a/...mponents/features/dataset-creation/configuration/questions/DatasetConfigurationRating.vue b/...mponents/features/dataset-creation/configuration/questions/DatasetConfigurationRating.vue
@@ -1,46 +1,80 @@
 <template>
-  <div class="dataset-config-rating__input-container">
-    <input
-      type="number"
-      min="0"
-      max="10"
-      step="1"
-      :value="value.length - 1"
-      @input="onInput($event.target.value)"
-      @focus="$emit('is-focused', true)"
-      @blur="$emit('is-focused', false)"
-      :placeholder="placeholder"
-      class="dataset-config-rating__input"
-    />
+  <div>
+    <div
+      :class="{ '--error': errors.length }"
+      class="dataset-config-rating__input-container"
+    >
+      <input
+        type="number"
+        min="0"
+        max="10"
+        step="1"
+        :value="
+          question.settings.options.length
+            ? question.settings.options.length - 1
+            : 0
+        "
+        @input="onInput($event.target.value)"
+        @focus.stop="onFocus"
+        @blur="onBlur"
+        :placeholder="placeholder"
+        class="dataset-config-rating__input"
+      />
+    </div>
+    <Validation v-if="errors.length" :validations="translatedValidations" />
   </div>
 </template>
 
 <script>
 export default {
+  data() {
+    return {
+      errors: [],
+      isDirty: false,
+    };
+  },
   props: {
-    value: {
-      type: Array,
+    question: {
+      type: Object,
       required: true,
     },
     placeholder: {
       type: String,
       default: "",
     },
   },
-  model: {
-    prop: "value",
-    event: "on-value-change",
+  computed: {
+    translatedValidations() {
+      return this.errors.map((validation) => {
+        return this.$t(validation);
+      });
+    },
   },
   methods: {
+    validateOptions() {
+      this.errors = this.question.validate();
+    },
+    onFocus() {
+      this.$emit("is-focused", true);
+    },
+    onBlur() {
+      this.isDirty = true;
+      this.validateOptions();
+      this.$emit("is-focused", false);
+    },
     onInput(inputValue) {
-      const valuesArray = Array.from(
-        { length: parseInt(inputValue) + 1 },
-        (_, i) => ({
-          value: i,
-        })
-      );
+      let value = parseInt(inputValue);
+      value = Math.max(1, Math.min(value, 10));
+
+      const valuesArray = Array.from({ length: value + 1 }, (_, i) => ({
+        value: i,
+      }));
+
+      this.question.settings.options = valuesArray;
 
-      this.$emit("on-value-change", valuesArray);
+      if (this.isDirty) {
+        this.validateOptions();
+      }
     },
   },
 };
@@ -65,6 +99,7 @@ export default {
   width: 100%;
   outline: none;
   color: var(--fg-secondary);
+  @include font-size(12px);
   @include input-placeholder {
     color: var(--fg-tertiary);
   }

diff --git a/...ents/features/dataset-creation/configuration/shared/DatasetConfigurationChipsSelector.vue b/...ents/features/dataset-creation/configuration/shared/DatasetConfigurationChipsSelector.vue
@@ -56,7 +56,7 @@ export default {
   &__options {
     display: flex;
     flex-wrap: wrap;
-    gap: $base-space;
+    gap: $base-space - 1px;
     padding: 0;
     margin: $base-space 0 0;
   }

diff --git a/argilla-frontend/pages/new/_id.vue b/argilla-frontend/pages/new/_id.vue
@@ -0,0 +1,14 @@
+<template></template>
+
+<script>
+export default {
+  middleware({ route, redirect }) {
+    if (route.params.id) {
+      const newRoute = encodeURIComponent(route.params.id);
+      redirect(newRoute);
+    } else {
+      redirect("/");
+    }
+  },
+};
+</script>
diff --git a/argilla-frontend/pages/useNewDatasetViewModel.ts b/argilla-frontend/pages/useNewDatasetViewModel.ts
@@ -1,15 +1,20 @@
 import { useResolve } from "ts-injecty";
-import { ref, useRoute } from "@nuxtjs/composition-api";
+import { ref, useRoute, useContext } from "@nuxtjs/composition-api";
 import { GetDatasetCreationUseCase } from "~/v1/domain/usecases/get-dataset-creation-use-case";
 
 export const useNewDatasetViewModel = () => {
+  const { error } = useContext();
   const datasetConfig = ref();
   const getDatasetCreationUseCase = useResolve(GetDatasetCreationUseCase);
 
   const getNewDatasetByRepoId = async (repositoryId: string) => {
-    datasetConfig.value = await getDatasetCreationUseCase.execute(
-      decodeURI(repositoryId)
-    );
+    try {
+      datasetConfig.value = await getDatasetCreationUseCase.execute(
+        repositoryId
+      );
+    } catch (e) {
+      error({ statusCode: 404, message: "Cannot fetch the dataset" });
+    }
   };
 
   const getNewDatasetByRepoIdFromUrl = async () => {

diff --git a/argilla-frontend/plugins/directives/required-field.directive.js b/argilla-frontend/plugins/directives/required-field.directive.js
@@ -5,22 +5,19 @@ import Vue from "vue";
 //    => color (String) the color of the asterisk : black by default
 
 Vue.directive("required-field", {
-  bind: (element, binding, node) => {
-    if (binding?.value) {
-      const { color, show } = binding?.value ?? { show: true, color: "black" };
-
-      if (!show) return;
-
-      const text = document.createTextNode(" *");
-      const textWrapper = document.createElement("span");
-      textWrapper.setAttribute("title", "Required response");
-      textWrapper.setAttribute("role", "mark");
-      textWrapper.style.color = color;
-      textWrapper.appendChild(text);
-
-      node.context.$nextTick(() => {
-        element.insertAdjacentElement("afterEnd", textWrapper);
-      });
+  bind(element, binding) {
+    const span = document.createElement("span");
+    span.textContent = " *";
+    span.style.color = binding.value.color;
+    span.setAttribute("title", "Required response");
+    span.setAttribute("role", "mark");
+    element.appendChild(span);
+    span.style.display = binding.value.show ? "inline" : "none";
+  },
+  update(element, binding) {
+    const span = element.querySelector("span");
+    if (span) {
+      span.style.display = binding.value.show ? "inline" : "none";
     }
   },
 });
diff --git a/argilla-frontend/translation/en.js b/argilla-frontend/translation/en.js
@@ -282,6 +282,9 @@ export default {
         atLeastTwoOptions: "At least two options are required",
         optionsWithoutLabel: "Empty options are not allowed",
       },
+      rating: {
+        atLeastTwoOptions: "At least two options are required",
+      },
     },
     atLeastOneQuestion: "At least one question is required.",
     atLeastOneRequired: "At least one required question is needed.",
@@ -327,6 +330,14 @@ export default {
       span: "Span",
       "no mapping": "No mapping",
     },
+    questionId: {
+      text: "text",
+      rating: "rating",
+      label_selection: "label",
+      ranking: "ranking",
+      multi_label_selection: "multi-label",
+      span: "span",
+    },
   },
   persistentStorage: {
     adminOrOwner:

diff --git a/argilla-frontend/translation/es.js b/argilla-frontend/translation/es.js
@@ -280,6 +280,9 @@ export default {
         atLeastTwoOptions: "Se requieren al menos dos opciones",
         optionsWithoutLabel: "No se permiten opciones vacías",
       },
+      rating: {
+        atLeastTwoOptions: "Se requieren al menos dos opciones",
+      },
     },
     atLeastOneQuestion: "Se requiere al menos una pregunta.",
     atLeastOneRequired: "Se requiere al menos una pregunta obligatoria.",

diff --git a/argilla-frontend/v1/domain/entities/hub/QuestionCreation.ts b/argilla-frontend/v1/domain/entities/hub/QuestionCreation.ts
@@ -64,6 +64,10 @@ export class QuestionCreation {
     this.required = true;
   }
 
+  get isRequired(): boolean {
+    return this.required;
+  }
+
   get isTextType(): boolean {
     return this.type.isTextType;
   }
@@ -111,6 +115,11 @@ export class QuestionCreation {
         );
       }
     }
+    if (this.isRatingType) {
+      if (this.options.length < 2) {
+        errors.push("datasetCreation.questions.rating.atLeastTwoOptions");
+      }
+    }
 
     return errors;
   }