fix(task): make the whole task clickable #3176

konrad · 2023-02-27T16:48:24Z

konrad commented

2023-02-27 16:48:24 +00:00

As discussed, this PR uses the approach from here to make the whole tasks in a list of tasks clickable.

Resolves #3172

As discussed, this PR uses the approach from [here](https://css-tricks.com/block-links-the-search-for-a-perfect-solution/) to make the whole tasks in a list of tasks clickable. Resolves #3172

konrad added 3 commits 2023-02-27 16:48:25 +00:00

00248e7c91

Revert "fix(task): allow clicking on the whole task to open the task detail view"

This reverts commit fe764a46e9.

5e9ed290ac

fix(task): don't wrap the whole task item in an a element

continuous-integration/drone/pr Build is failing Details

4924470f2b

fix(task): task link focus style

konrad requested review from dpschen 2023-02-27 16:48:27 +00:00

konrad reviewed 2023-02-27 16:50:12 +00:00

src/components/tasks/partials/singleTaskInList.vue

						
				@ -4,3 +2,1 @@

						:class="{'is-loading': taskService.loading}"

						class="task loader-container"

					>

					<div :class="{'is-loading': taskService.loading}" class="task loader-container" @click.stop.self="openTaskDetail">

konrad commented

2023-02-27 16:49:23 +00:00

This is probably not an ideal solution but I was unable to make it work without the limit to .self. It would always trigger when using the checkbox to mark a task as done.

This is probably not an ideal solution but I was unable to make it work without the limit to `.self`. It would always trigger when using the checkbox to mark a task as done.

src/components/tasks/partials/singleTaskInList.vue

						
				@ -278,0 +279,4 @@

				function openTaskDetail() {

					const isTextSelected = window.getSelection().toString()

					if (!isTextSelected) {

						router.push(taskDetailRoute.value)

konrad commented

2023-02-27 16:50:05 +00:00

Because just calling click() on the a element itself would trigger this event handler again I restored to pushing the route directly.

Because just calling `click()` on the `a` element itself would trigger this event handler again I restored to pushing the route directly.

konrad referenced this pull request

2023-02-27 16:50:28 +00:00

style(list): highlighted area of items is not entirely clickable #2016

dpschen commented

2023-02-28 10:15:49 +00:00

I'm not so happy with this solution either since we still have nested links / buttons.

I tried to rework this some time last year but never pushed the branch since there were some problems. ~~I don't remember what exactly was the issue but I think it was more a merge issue / I realised I should work on something more important than this change instead.~~ Edit: I remembered now: we were moving the whole project to vues script setup. That branch is still based on object components. A rebase might be possible though.

Maybe we could solve this issue based on that branch. It might even be that I solved the 'whole task clickable' issue it in that branch already—I don't remember.

I'm not so happy with this solution either since we still have nested links / buttons. I tried to rework this some time last year but never pushed the branch since there were some problems. ~~I don't remember what exactly was the issue but I think it was more a merge issue / I realised I should work on something more important than this change instead.~~ **Edit:** I remembered now: we were moving the whole project to vues script setup. That branch is still based on object components. A rebase might be possible though. Maybe we could solve this issue based on that branch. It might even be that I solved the 'whole task clickable' issue it in that branch already—I don't remember.

konrad commented

2023-02-28 10:28:44 +00:00

I'm not so happy with this solution either since we still have nested links / buttons.

Okay so better do this once and for all?

I only tried to fix the "only the task title is clickable" problem but you're right

> I'm not so happy with this solution either since we still have nested links / buttons. Okay so better do this once and for all? I only tried to fix the "only the task title is clickable" problem but you're right

dpschen commented

2023-02-28 14:55:23 +00:00

Okay so better do this once and for all?

That was the idea. Maybe we should even abstract the part with the clickable background. Because we could use the same code also in the sidebar and for the namespace/project teasers.

> Okay so better do this once and for all? That was the idea. Maybe we should even abstract the part with the clickable background. Because we could use the same code also in the sidebar and for the namespace/project teasers.

WofWca reviewed 2023-03-21 13:49:41 +00:00

src/components/input/fancycheckbox.vue

						
				@ -7,7 +7,7 @@

							@change="(event: Event) => updateData((event.target as HTMLInputElement).checked)"

							type="checkbox"

						/>

						<label :for="checkBoxId" class="check" @click.prevent="check">

WofWca commented

2023-03-21 13:49:41 +00:00

FYI @click.prevent="check" doesn't respect the checkbox being disabled, so yep, needs to be removed. Perhaps in a separate MR in case we don't have enough time for this one.

FYI `@click.prevent="check"` doesn't respect the checkbox being disabled, so yep, needs to be removed. Perhaps in a separate MR in case we don't have enough time for this one.

👍 1