Skip to content

Commit

Permalink
Dataset Creation UI fixes & Improvements (#5670)
Browse files Browse the repository at this point in the history
- [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 <francis@argilla.io>
Co-authored-by: Paco Aranda <frascuchon@gmail.com>
Co-authored-by: José Francisco Calvo <jose@argilla.io>
Co-authored-by: José Francisco Calvo <josefranciscocalvo@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
6 people authored Nov 8, 2024
1 parent 940bd40 commit 783c7c0
Show file tree
Hide file tree
Showing 15 changed files with 181 additions and 66 deletions.
3 changes: 3 additions & 0 deletions argilla-frontend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -223,7 +249,6 @@ export default {
}
&__ghost {
opacity: 0.5;
background: lime;
}
&__selector {
&__intro {
Expand All @@ -247,8 +272,5 @@ export default {
justify-content: center;
}
}
.flip-list-move {
transition: transform 0.3s;
}
}
</style>
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
},
},
};
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
14 changes: 14 additions & 0 deletions argilla-frontend/pages/new/_id.vue
Original file line number Diff line number Diff line change
@@ -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>
13 changes: 9 additions & 4 deletions argilla-frontend/pages/useNewDatasetViewModel.ts
Original file line number Diff line number Diff line change
@@ -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 () => {
Expand Down
29 changes: 13 additions & 16 deletions argilla-frontend/plugins/directives/required-field.directive.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
},
});
11 changes: 11 additions & 0 deletions argilla-frontend/translation/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions argilla-frontend/translation/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
9 changes: 9 additions & 0 deletions argilla-frontend/v1/domain/entities/hub/QuestionCreation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ export class QuestionCreation {
this.required = true;
}

get isRequired(): boolean {
return this.required;
}

get isTextType(): boolean {
return this.type.isTextType;
}
Expand Down Expand Up @@ -111,6 +115,11 @@ export class QuestionCreation {
);
}
}
if (this.isRatingType) {
if (this.options.length < 2) {
errors.push("datasetCreation.questions.rating.atLeastTwoOptions");
}
}

return errors;
}
Expand Down
Loading

0 comments on commit 783c7c0

Please sign in to comment.