[settings] Use new components#393
Conversation
|
@marcoambrosini Can you guide us with this? Does this more or less covers the requirements of nextcloud/server#55667 ? |
|
@julien-nc I think everything should wrap to the same width of the new components. Copying @kra-mo for further input |
|
@marcoambrosini Done. Updated the initial comment. |
lukasdotcom
left a comment
There was a problem hiding this comment.
LGTM except for something unrelated I noticed could be improved.
| <NcFormBoxSwitch v-if="state.assistant_available" | ||
| :model-value="state.assistant_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'assistant_enabled')"> | ||
| {{ t('assistant', 'Enable Nextcloud Assistant in header') }} | ||
| </div> | ||
| </NcCheckboxRadioSwitch> | ||
| <NcCheckboxRadioSwitch v-if="state.audio_chat_available" | ||
| :model-value="state.autoplay_audio_chat" | ||
| @update:model-value="onCheckboxChanged($event, 'autoplay_audio_chat')"> | ||
| <div class="checkbox-text"> | ||
| </NcFormBoxSwitch> | ||
| <NcFormBoxSwitch v-if="state.audio_chat_available" | ||
| :model-value="state.autoplay_audio_chat" | ||
| @update:model-value="onCheckboxChanged($event, 'autoplay_audio_chat')"> | ||
| {{ t('assistant', 'Auto-play audio chat responses') }} | ||
| </div> | ||
| </NcCheckboxRadioSwitch> | ||
| <NcCheckboxRadioSwitch v-if="state.free_prompt_picker_available" | ||
| :model-value="state.free_prompt_picker_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'free_prompt_picker_enabled')"> | ||
| <div class="checkbox-text"> | ||
| </NcFormBoxSwitch> | ||
| <NcFormBoxSwitch v-if="state.free_prompt_picker_available" | ||
| :model-value="state.free_prompt_picker_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'free_prompt_picker_enabled')"> | ||
| {{ t('assistant', 'Enable AI text generation in smart picker') }} | ||
| </div> | ||
| </NcCheckboxRadioSwitch> | ||
| <NcCheckboxRadioSwitch v-if="state.text_to_image_picker_available" | ||
| :model-value="state.text_to_image_picker_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'text_to_image_picker_enabled')"> | ||
| <div class="checkbox-text"> | ||
| </NcFormBoxSwitch> | ||
| <NcFormBoxSwitch v-if="state.text_to_image_picker_available" | ||
| :model-value="state.text_to_image_picker_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'text_to_image_picker_enabled')"> | ||
| {{ t('assistant', 'Enable AI image generation in smart picker') }} | ||
| </div> | ||
| </NcCheckboxRadioSwitch> | ||
| <NcCheckboxRadioSwitch v-if="state.text_to_sticker_picker_available" | ||
| :model-value="state.text_to_sticker_picker_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'text_to_sticker_picker_enabled')"> | ||
| <div class="checkbox-text"> | ||
| </NcFormBoxSwitch> | ||
| <NcFormBoxSwitch v-if="state.text_to_sticker_picker_available" | ||
| :model-value="state.text_to_sticker_picker_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'text_to_sticker_picker_enabled')"> | ||
| {{ t('assistant', 'Enable AI sticker generation in smart picker') }} | ||
| </div> | ||
| </NcCheckboxRadioSwitch> | ||
| <NcCheckboxRadioSwitch v-if="state.speech_to_text_picker_available" | ||
| :model-value="state.speech_to_text_picker_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'speech_to_text_picker_enabled')"> | ||
| <div class="checkbox-text"> | ||
| </NcFormBoxSwitch> | ||
| <NcFormBoxSwitch v-if="state.speech_to_text_picker_available" | ||
| :model-value="state.speech_to_text_picker_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'speech_to_text_picker_enabled')"> | ||
| {{ t('assistant', 'Enable AI transcription in smart picker') }} | ||
| </div> | ||
| </NcCheckboxRadioSwitch> | ||
| <div v-if="noProvidersAvailable" class="settings-hint"> | ||
| <InformationOutlineIcon class="icon" /> | ||
| <span> | ||
| {{ t('assistant', 'No suitable providers are available. They must first be enabled by your administrator.') }} | ||
| </span> | ||
| </div> | ||
| </NcFormBoxSwitch> |
There was a problem hiding this comment.
All the NcFormBox* components are supposed to be used inside the <NcFormBox> component.(maybe not clear from the documentation, but follows Vue naming convention).
It makes it visually grouped with the rounded box effect and a small gap.
There was a problem hiding this comment.
Are you saying it's fine to have a <NcFormGroup> wrapping a <NcFormGroup> wrapping <NcFormBox*> like this?
<NcFormGroup>
<NcFormBox>
<NcFormBoxSwitch />
</NcFormBox>
</NcFormGroup>| <NcFormBoxSwitch :model-value="state.assistant_enabled" | ||
| @update:model-value="onCheckboxChanged($event, 'assistant_enabled')"> | ||
| {{ t('assistant', 'Enable Nextcloud Assistant in header') }} | ||
| </div> | ||
| </NcCheckboxRadioSwitch> | ||
| <NcNoteCard v-if="!state.text_processing_available" type="warning"> | ||
| {{ t('assistant', 'To be able to use this feature, please install at least one AI task processing provider.') }} | ||
| </NcNoteCard> | ||
| <NcCheckboxRadioSwitch | ||
| :model-value="state.free_prompt_picker_enabled" | ||
| :disabled="!state.free_prompt_task_type_available" | ||
| @update:model-value="onCheckboxChanged($event, 'free_prompt_picker_enabled')"> | ||
| <div class="checkbox-text"> | ||
| </NcFormBoxSwitch> |
There was a problem hiding this comment.
Might be less important for a single item, but better to still put it into the box to follow the structure.
There was a problem hiding this comment.
Never mind, I miss looking at the collapsed diff view on GitHub.
There was a problem hiding this comment.
However, NcNoteCard inside NcFormBox does look a bit off because of the extra margin it adds.
But I'll leave it to designers 👀
There was a problem hiding this comment.
But I'll leave it to designers 👀
Yeah, no haha. I wouldn't put it in the box with such margins.
…issues Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
2178283 to
5e03dab
Compare
kra-mo
left a comment
There was a problem hiding this comment.
What I said, and the boxes. But I'll let you developers sort that out :)
| </ul> | ||
| </div> | ||
| </NcNoteCard> | ||
| <NcFormGroup :label="t('assistant', 'Select which features you want to enable')" |
There was a problem hiding this comment.
This is very redundant. Can we kill it? :D
Just label the group "Assistant features" or something for a11y and hide the label.
There was a problem hiding this comment.
If you don't want to change strings with this PR, that is also fine, just hide it for now.
There was a problem hiding this comment.
Nevermind, I can see you changed other strings.
There was a problem hiding this comment.
A general note to labelling — it should be the name of a thing but not an instruction to the end user. Even if it is hidden, because for a11y using assistive technology with "Assistant features landmark" or "Assistant features group" is easier than "Select which features you want to enable landmark".
A long instruction if needed can be added to the description.
…ve switch labels Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
|
The screenshots in the initial comment have been updated. |
|
Thanks @kra-mo @ShGKme @julien-nc ! Looks great now :) |


refs #385
And fix some style issues.