feat: hide quick add magic help behind a tooltip #3353

Merged
konrad merged 13 commits from feature/hide-quick-add-magic-help into main 2023-06-05 15:02:43 +00:00
6 changed files with 51 additions and 22 deletions

View File

@ -67,6 +67,7 @@ import {
faStar,
faSun,
faTimesCircle,
faCircleQuestion,
} from '@fortawesome/free-regular-svg-icons'
import {FontAwesomeIcon} from '@fortawesome/vue-fontawesome'
@ -86,6 +87,7 @@ library.add(faCheckDouble)
library.add(faChessKnight)
library.add(faChevronDown)
library.add(faCircleInfo)
library.add(faCircleQuestion)
library.add(faClock)
library.add(faCloudDownloadAlt)
library.add(faCloudUploadAlt)

View File

@ -1,7 +1,7 @@
<template>
<div class="task-add" ref="taskAdd">
<div class="add-task__field field is-grouped">
dpschen marked this conversation as resolved
Review

Add a square BaseButton with nice padding (full height of one text line) to the top right of the textarea wrapper div. It has an info "?" icon inside. It has a tooltip with the quick add magic hint from the expandable. The same message is also via aria-label on the BaseButton. On click it will open the explanation directly.

Reuse the hover logic to show and hide the info button.

If I'm not mistaken the quickaddmagic.vue will be obsolete.

Add a square BaseButton with nice padding (full height of one text line) to the top right of the textarea wrapper div. It has an info "?" icon inside. It has a tooltip with the quick add magic hint from the expandable. The same message is also via `aria-label` on the BaseButton. On click it will open the explanation directly. Reuse the hover logic to show and hide the info button. If I'm not mistaken the `quickaddmagic.vue` will be obsolete.
Review

That makes sense.

That makes sense.
Review

I've now changed it so that it does what you're describing, but kept it in the existing component. That way the modal (and all logic to show it) stays in there and does not pollute the add task component.

I've now changed it so that it does what you're describing, but kept it in the existing component. That way the modal (and all logic to show it) stays in there and does not pollute the add task component.
Review

The new version doesn't hide the "?" icon when the add-task is not hovered or focused.

To simplify the logic the delayEnter option of useElementHover can be used (didn't exist before).

I guess this could still be abstracted via a slot.
This way we would have an info component that shows the '?' always when focused or hovered.

The new version doesn't hide the "?" icon when the add-task is not hovered or focused. To simplify the logic the `delayEnter` option of [useElementHover](https://vueuse.org/core/useElementHover/) can be used (didn't exist before). I guess this could still be abstracted via a slot. This way we would have an info component that shows the '?' always when focused or hovered.
Review

Contrast of the icon is really bad

Contrast of the icon is really bad
Review

I've improved the contrast

I've improved the contrast
Review

The new version doesn't hide the "?" icon when the add-task is not hovered or focused.

IMHO it's fine to leave it like that. It's not in the way and would avoid flickering of another element when focusing

> The new version doesn't hide the "?" icon when the add-task is not hovered or focused. IMHO it's fine to leave it like that. It's not in the way and would avoid flickering of another element when focusing
Review

[…] would avoid flickering of another element when focusing.

Fine with me. Flickering would be 'subtle' though, because I imagined a transition.

Now I think that the default is too strongly visible. What I head in mind was an alignment with e.g. the hover for lists / projects in the SingleLineInList task.

The color also needs a defined transition. If we add that transition we should add it for the icon in the front as well.

> […] would avoid flickering of another element when focusing. Fine with me. Flickering would be 'subtle' though, because I imagined a transition. Now I think that the default is too strongly visible. What I head in mind was an alignment with e.g. the hover for lists / projects in the SingleLineInList task. The color also needs a defined transition. If we add that transition we should add it for the icon in the front as well.
Review

Added a transition, seems to reduce the flickering by a lot. I also added a hover effect to highlight the icon when the input field is hovered.

Added a transition, seems to reduce the flickering by a lot. I also added a hover effect to highlight the icon when the input field is hovered.
Review

added a hover effect to highlight the icon when the input field is hovered.

Only seems to work if the field is focused

> added a hover effect to highlight the icon when the input field is hovered. Only seems to work if the field is focused
Review

It works for me when the input is not focused. Which browser and version are you testing in?

It works for me when the input is not focused. Which browser and version are you testing in?
Review

Might be that I didn't test the latest version. Works now

Might be that I didn't test the latest version. Works now
<p class="control has-icons-left is-expanded">
<p class="control has-icons-left has-icons-right is-expanded">
<textarea
class="add-task-textarea input"
:class="{'textarea-empty': newTaskTitle === ''}"
@ -16,6 +16,7 @@
<span class="icon is-small is-left">
<icon icon="tasks"/>
</span>
<quick-add-magic :highlight-hint-icon="taskAddHovered"/>
</p>
<p class="control">
<x-button
@ -32,11 +33,10 @@
</x-button>
</p>
</div>
<Expandable :open="errorMessage !== '' || taskAddFocused || taskAddHovered && debouncedTaskAddHovered">
<Expandable :open="errorMessage !== ''">
<p class="pt-3 mt-0 help is-danger" v-if="errorMessage !== ''">
{{ errorMessage }}
</p>
<quick-add-magic v-else class="quick-add-magic" />
</Expandable>
</div>
</template>
@ -44,7 +44,7 @@
<script setup lang="ts">
import {computed, ref} from 'vue'
import {useI18n} from 'vue-i18n'
import {refDebounced, useElementHover, useFocusWithin} from '@vueuse/core'
import {useElementHover} from '@vueuse/core'
import {RELATION_KIND} from '@/types/IRelationKind'
import type {ITask} from '@/modelTypes/ITask'
@ -77,8 +77,6 @@ const {t} = useI18n({useScope: 'global'})
const authStore = useAuthStore()
const taskStore = useTaskStore()
const taskAdd = ref<HTMLTextAreaElement | null>(null)
// enable only if we don't have a modal
// onStartTyping(() => {
// if (newTaskInput.value === null || document.activeElement === newTaskInput.value) {
@ -87,10 +85,8 @@ const taskAdd = ref<HTMLTextAreaElement | null>(null)
// newTaskInput.value.focus()
// })
const { focused: taskAddFocused } = useFocusWithin(taskAdd)
const taskAdd = ref<HTMLTextAreaElement | null>(null)
const taskAddHovered = useElementHover(taskAdd)
const debouncedTaskAddHovered = refDebounced(taskAddHovered, 500)
const errorMessage = ref('')
@ -244,7 +240,14 @@ defineExpose({
text-overflow: ellipsis;
}
.quick-add-magic {
padding-top: 0.75rem;
.control.has-icons-left .icon,
.control.has-icons-right .icon {
transition: all $transition;
}
</style>
<style>
button.show-helper-text {
right: 0;
}
</style>

View File

@ -1,9 +1,14 @@
<template>
<div v-if="mode !== 'disabled' && prefixes !== undefined">
<p class="help has-text-grey">
{{ $t('task.quickAddMagic.hint') }}.
<ButtonLink @click="() => visible = true">{{ $t('task.quickAddMagic.what') }}</ButtonLink>
</p>
<template v-if="mode !== 'disabled' && prefixes !== undefined">
dpschen marked this conversation as resolved Outdated

Remove wrapper div and use template instead

Remove wrapper div and use template instead

Ah, I thought you couldn't use a template as root node. TIL.
I guess that changed in vue 3?

Ah, I thought you couldn't use a `template` as root node. TIL. I guess that changed in vue 3?

Now changed

Now changed

Ah, I thought you couldn't use a template as root node. TIL.
I guess that changed in vue 3?

Unsure, but yes might be!

> Ah, I thought you couldn't use a `template` as root node. TIL. > I guess that changed in vue 3? Unsure, but yes might be!
<BaseButton
@click="() => visible = true"
class="icon is-small show-helper-text"
v-tooltip="$t('task.quickAddMagic.hint')"
dpschen marked this conversation as resolved Outdated

Remove extra () =>

Remove extra `() =>`
:aria-label="$t('task.quickAddMagic.hint')"
:class="{'is-highlighted': highlightHintIcon}"
>
<icon :icon="['far', 'circle-question']"/>
</BaseButton>
<modal
:enabled="visible"
@close="() => visible = false"
@ -69,7 +74,7 @@
<li>17th ({{ $t('task.quickAddMagic.dateNth', {day: '17'}) }})</li>
</ul>
<p>{{ $t('task.quickAddMagic.dateTime', {time: 'at 17:00', timePM: '5pm'}) }}</p>
<h3>{{ $t('task.quickAddMagic.repeats') }}</h3>
<p>{{ $t('task.quickAddMagic.repeatsDescription', {suffix: 'every {amount} {type}'}) }}</p>
<p>{{ $t('misc.forExample') }}</p>
@ -86,13 +91,13 @@
</ul>
</card>
</modal>
</div>
</template>
</template>
<script setup lang="ts">
import {ref, computed} from 'vue'
import ButtonLink from '@/components/misc/ButtonLink.vue'
import BaseButton from '@/components/base/BaseButton.vue'
import {getQuickAddMagicMode} from '@/helpers/quickAddMagicMode'
import {PREFIXES} from '@/modules/parseTaskText'
@ -100,5 +105,20 @@ import {PREFIXES} from '@/modules/parseTaskText'
const visible = ref(false)
const mode = ref(getQuickAddMagicMode())
defineProps<{
highlightHintIcon: boolean,
}>()
const prefixes = computed(() => PREFIXES[mode.value])
</script>
<style lang="scss" scoped>
.show-helper-text {
// Bulma adds pointer-events: none to the icon so we need to override it back here.
konrad marked this conversation as resolved Outdated

Positioning should happen from outside. Remove right: 0 from here

Positioning should happen from outside. Remove `right: 0` from here

Done

Done

I had to use an unscoped style for this because I could not add a new class to the quick-add-magic component due to it being multi-node. Not sure if that's the best solution.

I had to use an unscoped style for this because I could not add a new class to the `quick-add-magic` component due to it being multi-node. Not sure if that's the best solution.
pointer-events: auto !important;
konrad marked this conversation as resolved Outdated

Why pointer-events: auto?

Why `pointer-events: auto`?

Because in BaseButton we set that to none which makes the button unclickable (and unhoverable).

Because in `BaseButton` we set that to `none` which makes the button unclickable (and unhoverable).
That can't be it, because there it's set to `auto` as well: https://kolaente.dev/vikunja/frontend/src/branch/main/src/components/base/BaseButton.vue#L115

Seems to be the .control.has-icons-left .icon class of bulma. Can you add a comment?

It also seems that the textarea could profit from has-icon-right so that there is enough padding for the button.

Seems to be the `.control.has-icons-left .icon` class of bulma. Can you add a comment? It also seems that the textarea could profit from `has-icon-right` so that there is enough padding for the button.

Added a comment and more padding.

Added a comment and more padding.
}
dpschen marked this conversation as resolved Outdated

Is nesting even necessary if we already need to use !important for inheriting the color?

Is nesting even necessary if we already need to use `!important` for inheriting the color?

It isn't, I added the nesting out of habit. Removed it now.

It isn't, I added the nesting out of habit. Removed it now.
.is-highlighted {
color: inherit !important;
}
</style>

View File

@ -737,8 +737,7 @@
"invalidAmount": "Please enter more than 0."
},
"quickAddMagic": {
"hint": "You can use Quick Add Magic",
"what": "What?",
"hint": "Use magic prefixes to define due dates, assignees and other task properties.",
"title": "Quick Add Magic",
"intro": "When creating a task, you can use special keywords to directly add attributes to the newly created task. This allows to add commonly used attributes to tasks much faster.",
"multiple": "You can use this multiple times.",

View File

@ -57,7 +57,7 @@
}
.field.has-addons .control .select select {
height: 100%;
height: 100%;
}
.control.has-icons-left .icon,

View File

@ -301,4 +301,9 @@ function prepareFiltersAndLoadTasks() {
border: none;
box-shadow: none;
}
.control.has-icons-left .icon,
.control.has-icons-right .icon {
transition: all $transition;
}
</style>