From 79ecd9afc0dd5f676f39460d4e2b378b2d4ea827 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Wed, 31 Jan 2024 22:50:38 +0100
Subject: [PATCH] fix(NcListItem): Do not use `a` as wrapper to prevent invalid
 HTML due to use of button within anchor

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 src/components/NcListItem/NcListItem.vue | 167 +++++++++++++----------
 1 file changed, 94 insertions(+), 73 deletions(-)

diff --git a/src/components/NcListItem/NcListItem.vue b/src/components/NcListItem/NcListItem.vue
index 92b4c8394c..972ca39783 100644
--- a/src/components/NcListItem/NcListItem.vue
+++ b/src/components/NcListItem/NcListItem.vue
@@ -248,66 +248,77 @@
 ```
 
 ### NcListItem compact mode
-```
-<ul style="width: 350px;">
-	<NcListItem
-		:name="'Name of the element'"
-		:counter-number="1"
-		:compact="true" >
-		<template #icon>
-			<div class="icon-edit" />
-		</template>
-		<template #subname>
-			This one is with subname
-		</template>
-		<template #actions>
-			<NcActionButton>
-				Button one
-			</NcActionButton>
-			<NcActionButton>
-				Button two
-			</NcActionButton>
-		</template>
-	</NcListItem>
-	<NcListItem
-		:name="'Name of the element'"
-		:compact="true" >
-		<template #icon>
-			<div class="icon-edit" />
-		</template>
-	</NcListItem>
-	<NcListItem
-		:name="'Name of the element'"
-		:counter-number="3"
-		:compact="true" >
-		<template #icon>
-			<div class="icon-edit" />
-		</template>
-		<template #subname>
-			This one is with subname
-		</template>
-		<template #actions>
-			<NcActionButton>
-				Button one
-			</NcActionButton>
-			<NcActionButton>
-				Button two
-			</NcActionButton>
-		</template>
-	</NcListItem>
-	<NcListItem
-		:name="'Name of the element'"
-		:compact="true"
-		:counter-number="4"
-		href="https://nextcloud.com">
-		<template #icon>
-			<div class="icon-edit" />
-		</template>
-		<template #subname>
-			This one is with an external link
-		</template>
-	</NcListItem>
-</ul>
+```vue
+<template>
+	<ul style="width: 350px;">
+		<NcListItem
+			:name="'Name of the element'"
+			:counter-number="1"
+			:compact="true" >
+			<template #icon>
+				<IconNoteText :size="20" />
+			</template>
+			<template #subname>
+				This one is with subname
+			</template>
+			<template #actions>
+				<NcActionButton>
+					Button one
+				</NcActionButton>
+				<NcActionButton>
+					Button two
+				</NcActionButton>
+			</template>
+		</NcListItem>
+		<NcListItem
+			:name="'Name of the element'"
+			:compact="true" >
+			<template #icon>
+				<IconNoteText :size="20" />
+			</template>
+		</NcListItem>
+		<NcListItem
+			:name="'Name of the element'"
+			:counter-number="3"
+			:compact="true" >
+			<template #icon>
+				<IconNoteText :size="20" />
+			</template>
+			<template #subname>
+				This one is with subname
+			</template>
+			<template #actions>
+				<NcActionButton>
+					Button one
+				</NcActionButton>
+				<NcActionButton>
+					Button two
+				</NcActionButton>
+			</template>
+		</NcListItem>
+		<NcListItem
+			:name="'Name of the element'"
+			:compact="true"
+			:counter-number="4"
+			href="https://nextcloud.com">
+			<template #icon>
+				<IconNoteText :size="20" />
+			</template>
+			<template #subname>
+				This one is with an external link
+			</template>
+		</NcListItem>
+	</ul>
+</template>
+<script>
+	import IconNoteText from 'vue-material-design-icons/NoteText.vue'
+
+	export default {
+		components: {
+			IconNoteText,
+		},
+	}
+</script>
 ```
 </docs>
 
@@ -320,13 +331,9 @@
 		:exact="to ? exact : null">
 		<li class="list-item__wrapper"
 			:class="{ 'list-item__wrapper--active' : isActive || active }">
-			<a :id="anchorId"
-				ref="list-item"
-				:href="routerLinkHref || href"
-				:target="target || (href === '#' ? undefined : '_blank')"
-				:rel="href === '#' ? undefined : 'noopener noreferrer'"
+			<div ref="list-item"
 				class="list-item"
-				:aria-label="linkAriaLabel"
+				tabindex="0"
 				@mouseover="handleMouseover"
 				@mouseleave="handleMouseleave"
 				@focus="handleFocus"
@@ -334,14 +341,20 @@
 				@keydown.tab.exact="handleTab"
 				@click="onClick($event, navigate, routerLinkHref)"
 				@keydown.esc="hideActions">
-
-				<div class="list-item-content__wrapper"
-					:class="{ 'list-item-content__wrapper--compact': compact }">
+				<div :class="['list-item-content__wrapper', {
+					'list-item-content__wrapper--compact': compact
+				}]">
 					<!-- @slot This slot is used for the NcAvatar or icon -->
 					<slot name="icon" />
 
 					<!-- Main content -->
-					<div class="list-item-content">
+					<a :id="anchorId || undefined"
+						tabindex="-1"
+						class="list-item-content"
+						:aria-label="linkAriaLabel"
+						:href="routerLinkHref || href"
+						:target="target || (href === '#' ? undefined : '_blank')"
+						:rel="href === '#' ? undefined : 'noopener noreferrer'">
 						<div class="list-item-content__main"
 							:class="{ 'list-item-content__main--oneline': oneLine }">
 
@@ -383,7 +396,8 @@
 								</span>
 							</div>
 						</div>
-					</div>
+					</a>
+
 					<!-- Actions -->
 					<div v-show="forceDisplayActions || displayActionsOnHoverFocus"
 						class="list-item-content__actions"
@@ -402,7 +416,7 @@
 				<div v-if="$slots.extra" class="list-item__extra">
 					<slot name="extra" />
 				</div>
-			</a>
+			</div>
 		</li>
 	</component>
 </template>
@@ -704,6 +718,7 @@ export default {
 <style lang="scss" scoped>
 
 .list-item__wrapper {
+	display: flex;
 	position: relative;
 	width: 100%;
 
@@ -738,10 +753,11 @@ export default {
 	flex: 0 0 auto;
 	justify-content: flex-start;
 	padding: 8px 10px;
+	// 4px padding for the focus-visible styles
+	margin: 4px;
 	// Fix for border-radius being too large for 3-line entries like in Mail
 	// 44px avatar size / 2 + 8px padding, and 2px for better visual quality
 	border-radius: 32px;
-	margin: 2px 0;
 	width: 100%;
 	cursor: pointer;
 	transition: background-color var(--animation-quick) ease-in-out;
@@ -753,6 +769,11 @@ export default {
 		background-color: var(--color-background-hover);
 	}
 
+	&:focus-visible {
+		outline: 2px solid var(--color-main-text);
+		box-shadow: 0 0 0 4px var(--color-main-background);
+	}
+
 	&-content__wrapper {
 		display: flex;
 		align-items: center;