Skip to content

Commit 783c7c0

Browse files
leiyrefrascuchonjfcalvopre-commit-ci[bot]
authored
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>
1 parent 940bd40 commit 783c7c0

File tree

15 files changed

+181
-66
lines changed

15 files changed

+181
-66
lines changed

argilla-frontend/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@ These are the section headers that we use:
1919
### Added
2020

2121
- Add a high-contrast theme & improvements for the forced-colors mode. ([#5661](https://github.com/argilla-io/argilla/pull/5661))
22+
- Added redirect to error page when repoId is invalid ([#5670](https://github.com/argilla-io/argilla/pull/5670))
2223

2324
### Fixed
2425

2526
- Fixed redirection problems after users sign-in using HF OAuth. ([#5635](https://github.com/argilla-io/argilla/pull/5635))
2627
- Fixed highlighting of the searched text in text and chat fields ([#5678](https://github.com/argilla-io/argilla/pull/5678))
28+
- Fixed validation for rating question when creating a dataset ([#5670](https://github.com/argilla-io/argilla/pull/5670))
29+
- Fixed question name based on question type when creating a dataset ([#5670](https://github.com/argilla-io/argilla/pull/5670))
2730

2831
## [2.4.0](https://github.com/argilla-io/argilla/compare/v2.3.0...v2.4.0)
2932

argilla-frontend/components/features/dataset-creation/configuration/DatasetConfigurationForm.vue

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,11 @@
3131
:group="{ name: 'fields' }"
3232
ghost-class="config-form__ghost"
3333
:disabled="isFocused"
34-
@start="drag = true"
35-
@end="drag = false"
3634
>
3735
<transition-group
3836
class="config-form__draggable-area-wrapper"
3937
type="transition"
40-
:name="!drag ? 'flip-list' : null"
38+
:css="false"
4139
>
4240
<DatasetConfigurationField
4341
v-for="field in dataset.selectedSubset.fields.filter(
@@ -87,7 +85,7 @@
8785
<transition-group
8886
class="config-form__draggable-area-wrapper"
8987
type="transition"
90-
:name="!drag ? 'flip-list' : null"
88+
:css="false"
9189
>
9290
<DatasetConfigurationQuestion
9391
v-for="question in dataset.selectedSubset.questions"
@@ -139,22 +137,50 @@ export default {
139137
return {
140138
isFocused: false,
141139
visibleDatasetCreationDialog: false,
142-
drag: false,
143140
};
144141
},
142+
computed: {
143+
getMaxNumberInNames() {
144+
return Math.max(
145+
...this.dataset.selectedSubset.questions.map((question) => {
146+
const numberInName = question.name.split("_").pop();
147+
return parseInt(numberInName) || 0;
148+
})
149+
);
150+
},
151+
},
145152
methods: {
146153
createDataset() {
147154
this.create(this.dataset);
148155
},
149-
addQuestion(type) {
150-
const questionName = `${type} ${this.dataset.selectedSubset.questions.length}`;
151-
this.dataset.selectedSubset.addQuestion(questionName, { type });
156+
generateName(type, number) {
157+
const typeName = this.$t(`config.questionId.${type}`);
158+
return `${typeName}_${parseInt(number) || 0}`;
152159
},
153-
onTypeIsChanged(name, type) {
154-
this.dataset.selectedSubset.addQuestion(name, {
155-
type: type.value,
160+
addQuestion(type) {
161+
const questionName = this.generateName(
162+
type,
163+
this.getMaxNumberInNames + 1
164+
);
165+
this.dataset.selectedSubset.addQuestion(questionName, {
166+
type,
156167
});
157168
},
169+
onTypeIsChanged(oldName, type) {
170+
const numberInName = oldName.split("_").pop();
171+
const index = this.dataset.selectedSubset.questions.findIndex(
172+
(q) => q.name === oldName
173+
);
174+
this.dataset.selectedSubset.removeQuestion(oldName);
175+
const newQuestionName = this.generateName(type.value, numberInName);
176+
this.dataset.selectedSubset.addQuestion(
177+
newQuestionName,
178+
{
179+
type: type.value,
180+
},
181+
index !== -1 ? index : undefined
182+
);
183+
},
158184
},
159185
setup() {
160186
return useDatasetConfigurationForm();
@@ -223,7 +249,6 @@ export default {
223249
}
224250
&__ghost {
225251
opacity: 0.5;
226-
background: lime;
227252
}
228253
&__selector {
229254
&__intro {
@@ -247,8 +272,5 @@ export default {
247272
justify-content: center;
248273
}
249274
}
250-
.flip-list-move {
251-
transition: transform 0.3s;
252-
}
253275
}
254276
</style>

argilla-frontend/components/features/dataset-creation/configuration/questions/DatasetConfigurationQuestion.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
/>
3030
<DatasetConfigurationRating
3131
v-else-if="question.settings.type.isRatingType"
32-
v-model="question.settings.options"
32+
:question="question"
3333
@is-focused="$emit('is-focused', $event)"
3434
/>
3535
<DatasetConfigurationRanking

argilla-frontend/components/features/dataset-creation/configuration/questions/DatasetConfigurationRanking.vue

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ export default {
9898
width: 100%;
9999
outline: none;
100100
color: var(--fg-secondary);
101+
@include font-size(12px);
101102
@include input-placeholder {
102103
color: var(--fg-tertiary);
103104
}

argilla-frontend/components/features/dataset-creation/configuration/questions/DatasetConfigurationRating.vue

Lines changed: 60 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,80 @@
11
<template>
2-
<div class="dataset-config-rating__input-container">
3-
<input
4-
type="number"
5-
min="0"
6-
max="10"
7-
step="1"
8-
:value="value.length - 1"
9-
@input="onInput($event.target.value)"
10-
@focus="$emit('is-focused', true)"
11-
@blur="$emit('is-focused', false)"
12-
:placeholder="placeholder"
13-
class="dataset-config-rating__input"
14-
/>
2+
<div>
3+
<div
4+
:class="{ '--error': errors.length }"
5+
class="dataset-config-rating__input-container"
6+
>
7+
<input
8+
type="number"
9+
min="0"
10+
max="10"
11+
step="1"
12+
:value="
13+
question.settings.options.length
14+
? question.settings.options.length - 1
15+
: 0
16+
"
17+
@input="onInput($event.target.value)"
18+
@focus.stop="onFocus"
19+
@blur="onBlur"
20+
:placeholder="placeholder"
21+
class="dataset-config-rating__input"
22+
/>
23+
</div>
24+
<Validation v-if="errors.length" :validations="translatedValidations" />
1525
</div>
1626
</template>
1727

1828
<script>
1929
export default {
30+
data() {
31+
return {
32+
errors: [],
33+
isDirty: false,
34+
};
35+
},
2036
props: {
21-
value: {
22-
type: Array,
37+
question: {
38+
type: Object,
2339
required: true,
2440
},
2541
placeholder: {
2642
type: String,
2743
default: "",
2844
},
2945
},
30-
model: {
31-
prop: "value",
32-
event: "on-value-change",
46+
computed: {
47+
translatedValidations() {
48+
return this.errors.map((validation) => {
49+
return this.$t(validation);
50+
});
51+
},
3352
},
3453
methods: {
54+
validateOptions() {
55+
this.errors = this.question.validate();
56+
},
57+
onFocus() {
58+
this.$emit("is-focused", true);
59+
},
60+
onBlur() {
61+
this.isDirty = true;
62+
this.validateOptions();
63+
this.$emit("is-focused", false);
64+
},
3565
onInput(inputValue) {
36-
const valuesArray = Array.from(
37-
{ length: parseInt(inputValue) + 1 },
38-
(_, i) => ({
39-
value: i,
40-
})
41-
);
66+
let value = parseInt(inputValue);
67+
value = Math.max(1, Math.min(value, 10));
68+
69+
const valuesArray = Array.from({ length: value + 1 }, (_, i) => ({
70+
value: i,
71+
}));
72+
73+
this.question.settings.options = valuesArray;
4274
43-
this.$emit("on-value-change", valuesArray);
75+
if (this.isDirty) {
76+
this.validateOptions();
77+
}
4478
},
4579
},
4680
};
@@ -65,6 +99,7 @@ export default {
6599
width: 100%;
66100
outline: none;
67101
color: var(--fg-secondary);
102+
@include font-size(12px);
68103
@include input-placeholder {
69104
color: var(--fg-tertiary);
70105
}

argilla-frontend/components/features/dataset-creation/configuration/shared/DatasetConfigurationChipsSelector.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export default {
5656
&__options {
5757
display: flex;
5858
flex-wrap: wrap;
59-
gap: $base-space;
59+
gap: $base-space - 1px;
6060
padding: 0;
6161
margin: $base-space 0 0;
6262
}

argilla-frontend/pages/new/_id.vue

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<template></template>
2+
3+
<script>
4+
export default {
5+
middleware({ route, redirect }) {
6+
if (route.params.id) {
7+
const newRoute = encodeURIComponent(route.params.id);
8+
redirect(newRoute);
9+
} else {
10+
redirect("/");
11+
}
12+
},
13+
};
14+
</script>

argilla-frontend/pages/useNewDatasetViewModel.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
import { useResolve } from "ts-injecty";
2-
import { ref, useRoute } from "@nuxtjs/composition-api";
2+
import { ref, useRoute, useContext } from "@nuxtjs/composition-api";
33
import { GetDatasetCreationUseCase } from "~/v1/domain/usecases/get-dataset-creation-use-case";
44

55
export const useNewDatasetViewModel = () => {
6+
const { error } = useContext();
67
const datasetConfig = ref();
78
const getDatasetCreationUseCase = useResolve(GetDatasetCreationUseCase);
89

910
const getNewDatasetByRepoId = async (repositoryId: string) => {
10-
datasetConfig.value = await getDatasetCreationUseCase.execute(
11-
decodeURI(repositoryId)
12-
);
11+
try {
12+
datasetConfig.value = await getDatasetCreationUseCase.execute(
13+
repositoryId
14+
);
15+
} catch (e) {
16+
error({ statusCode: 404, message: "Cannot fetch the dataset" });
17+
}
1318
};
1419

1520
const getNewDatasetByRepoIdFromUrl = async () => {

argilla-frontend/plugins/directives/required-field.directive.js

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,19 @@ import Vue from "vue";
55
// => color (String) the color of the asterisk : black by default
66

77
Vue.directive("required-field", {
8-
bind: (element, binding, node) => {
9-
if (binding?.value) {
10-
const { color, show } = binding?.value ?? { show: true, color: "black" };
11-
12-
if (!show) return;
13-
14-
const text = document.createTextNode(" *");
15-
const textWrapper = document.createElement("span");
16-
textWrapper.setAttribute("title", "Required response");
17-
textWrapper.setAttribute("role", "mark");
18-
textWrapper.style.color = color;
19-
textWrapper.appendChild(text);
20-
21-
node.context.$nextTick(() => {
22-
element.insertAdjacentElement("afterEnd", textWrapper);
23-
});
8+
bind(element, binding) {
9+
const span = document.createElement("span");
10+
span.textContent = " *";
11+
span.style.color = binding.value.color;
12+
span.setAttribute("title", "Required response");
13+
span.setAttribute("role", "mark");
14+
element.appendChild(span);
15+
span.style.display = binding.value.show ? "inline" : "none";
16+
},
17+
update(element, binding) {
18+
const span = element.querySelector("span");
19+
if (span) {
20+
span.style.display = binding.value.show ? "inline" : "none";
2421
}
2522
},
2623
});

argilla-frontend/translation/en.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,9 @@ export default {
282282
atLeastTwoOptions: "At least two options are required",
283283
optionsWithoutLabel: "Empty options are not allowed",
284284
},
285+
rating: {
286+
atLeastTwoOptions: "At least two options are required",
287+
},
285288
},
286289
atLeastOneQuestion: "At least one question is required.",
287290
atLeastOneRequired: "At least one required question is needed.",
@@ -327,6 +330,14 @@ export default {
327330
span: "Span",
328331
"no mapping": "No mapping",
329332
},
333+
questionId: {
334+
text: "text",
335+
rating: "rating",
336+
label_selection: "label",
337+
ranking: "ranking",
338+
multi_label_selection: "multi-label",
339+
span: "span",
340+
},
330341
},
331342
persistentStorage: {
332343
adminOrOwner:

0 commit comments

Comments
 (0)