Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup tag display for long lists of tags #3808

Merged
merged 11 commits into from Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
220 changes: 204 additions & 16 deletions frontend/src/components/VMediaInfo/VMediaTags.vue
@@ -1,32 +1,77 @@
<template>
<ul v-if="tags.length && additionalSearchViews" class="flex flex-wrap gap-3">
<VTag
v-for="(tag, index) in tags"
:key="index"
:href="localizedTagPath(tag)"
:title="tag.name"
/>
</ul>
<ul v-else class="flex flex-wrap gap-2">
<VMediaTag v-for="(tag, index) in tags" :key="index" tag="li">{{
tag.name
<div v-if="normalizedTags.length && additionalSearchViews" class="-my-1.5px">
<ul
ref="tagsContainerRef"
:aria-label="$t('mediaDetails.tags.title').toString()"
class="flex flex-wrap gap-3 overflow-y-hidden p-1.5px"
:class="heightClass"
>
zackkrida marked this conversation as resolved.
Show resolved Hide resolved
<li v-for="tag in visibleTags" :key="tag">
<VTag :href="localizedTagPath(tag)" :title="tag" />
</li>
</ul>
<VButton
v-if="hasOverflow"
size="small"
variant="transparent-tx"
has-icon-end
class="label-bold -ms-2 mt-4 hover:underline"
:aria-expanded="buttonStatus === 'show' ? 'false' : 'true'"
@click="handleClick"
>{{
$t(
buttonStatus === "show"
? "mediaDetails.tags.showMore"
: "mediaDetails.tags.showLess"
)
}}<VIcon
name="caret-down"
:size="4"
:class="{ '-scale-y-100 transform': buttonStatus === 'hide' }"
/></VButton>
</div>

<ul
v-else
class="flex flex-wrap gap-2"
:aria-label="$t('mediaDetails.tags').toString()"
>
<VMediaTag v-for="(tag, index) in normalizedTags" :key="index" tag="li">{{
tag
}}</VMediaTag>
</ul>
</template>
<script lang="ts">
import { computed, defineComponent, PropType } from "vue"
import {
computed,
defineComponent,
nextTick,
onMounted,
type PropType,
ref,
} from "vue"
import { useContext } from "@nuxtjs/composition-api"

import { useResizeObserver, watchDebounced } from "@vueuse/core"

import type { Tag } from "~/types/media"
import type { SupportedMediaType } from "~/constants/media"
import { useFeatureFlagStore } from "~/stores/feature-flag"
import { useSearchStore } from "~/stores/search"

import { focusElement } from "~/utils/focus-management"

import VMediaTag from "~/components/VMediaTag/VMediaTag.vue"
import VTag from "~/components/VTag/VTag.vue"
import VButton from "~/components/VButton.vue"
import VIcon from "~/components/VIcon/VIcon.vue"

// The number of rows to display before collapsing the tags
const ROWS_TO_DISPLAY = 3

export default defineComponent({
name: "VMediaTags",
components: { VMediaTag, VTag },
components: { VIcon, VButton, VMediaTag, VTag },
props: {
tags: {
type: Array as PropType<Tag[]>,
Expand All @@ -38,21 +83,164 @@ export default defineComponent({
},
},
setup(props) {
const tagsContainerRef = ref<HTMLElement>()

const searchStore = useSearchStore()
const featureFlagStore = useFeatureFlagStore()
const { app, $sendCustomEvent } = useContext()

const additionalSearchViews = computed(() =>
featureFlagStore.isOn("additional_search_views")
)

const localizedTagPath = (tag: Tag) => {
const localizedTagPath = (tag: string) => {
return searchStore.getCollectionPath({
type: props.mediaType,
collectionParams: { collection: "tag", tag: tag.name },
collectionParams: { collection: "tag", tag },
})
}

const normalizedTags = computed(() => {
return Array.from(new Set(props.tags.map((tag) => tag.name)))
})

const collapsibleRowsStartAt = ref<number>()
const dir = computed(() => {
return app.i18n.localeProperties.dir
})

function isFurtherInline(previous: HTMLElement, current: HTMLElement) {
if (dir.value === "rtl") {
return previous.offsetLeft < current.offsetLeft
}
return previous.offsetLeft > current.offsetLeft
}

function findRowStartsAt(parent: HTMLElement) {
const children = Array.from(parent.children)
if (!children.length) {
return 0
}
let rowCount = 0
for (let i = 0; i < children.length; i++) {
const child = children[i] as HTMLElement
const previous = child.previousElementSibling as HTMLElement
if (!previous) {
rowCount++
} else if (isFurtherInline(previous, child)) {
rowCount++
}
if (rowCount === ROWS_TO_DISPLAY + 1) {
return i
}
}
return children.length
}

/**
* Only the first 3 rows of tags are visible by default.
* If we hide the tags using CSS only, they will be tabbable,
* even though they are not visible.
*/
const visibleTags = computed<string[]>(() => {
return collapsibleRowsStartAt.value && buttonStatus.value === "show"
? normalizedTags.value.slice(0, collapsibleRowsStartAt.value)
: normalizedTags.value
})

const hasOverflow = computed(() => {
return (
collapsibleRowsStartAt.value &&
collapsibleRowsStartAt.value < normalizedTags.value.length
)
})

onMounted(() => {
/**
* Find the index of the first item after the third row of tags. This is used
* to determine which tags to hide.
*/
if (tagsContainerRef.value) {
collapsibleRowsStartAt.value = findRowStartsAt(tagsContainerRef.value)
}
})

const buttonStatus = ref<"show" | "hide">("show")
/**
* Toggles the text for the "Show more" button. When showing more tags, we also
* focus the first tag in the newly-opened row for a11y.
*/
const handleClick = () => {
buttonStatus.value = buttonStatus.value === "show" ? "hide" : "show"
$sendCustomEvent("TOGGLE_TAG_EXPANSION", {
toState: buttonStatus.value === "show" ? "collapsed" : "expanded",
})
if (buttonStatus.value === "hide" && collapsibleRowsStartAt.value) {
nextTick(() => {
if (!collapsibleRowsStartAt.value) {
return
}
const firstTagInFourthRow = tagsContainerRef.value?.children.item(
collapsibleRowsStartAt.value
) as HTMLElement
focusElement(firstTagInFourthRow?.querySelector("a"))
})
}
}

return { additionalSearchViews, localizedTagPath }
const heightClass = computed(() => {
if (!hasOverflow.value) {
return "max-h-none"
}
/**
* Height is 3 rows of tags, gaps, and a padding for the focus rings.
* 3 * 2rem (tags) + 2 * 0.75rem (2 gaps) + 0.1875rem (margin for the focus ring)
*/
return buttonStatus.value === "show" ? "max-h-[7.6875rem]" : "mah-h-none"
obulat marked this conversation as resolved.
Show resolved Hide resolved
})

const listWidth = ref<number>()
useResizeObserver(tagsContainerRef, (entries) => {
listWidth.value = entries[0].contentRect.width
})

watchDebounced(
listWidth,
(newWidth, oldWidth) => {
if (!tagsContainerRef.value) {
return
}
const isWidening = oldWidth && newWidth && newWidth > oldWidth

if (isWidening) {
collapsibleRowsStartAt.value = normalizedTags.value.length
}
nextTick(() => {
if (tagsContainerRef.value) {
collapsibleRowsStartAt.value = findRowStartsAt(
tagsContainerRef.value
)
}
})
},
{ debounce: 300 }
)

return {
tagsContainerRef,

additionalSearchViews,
localizedTagPath,

normalizedTags,
visibleTags,

hasOverflow,
buttonStatus,
heightClass,

handleClick,
}
},
})
</script>
5 changes: 5 additions & 0 deletions frontend/src/locales/scripts/en.json5
Expand Up @@ -498,6 +498,11 @@
},
imageInfo: "Image information",
audioInfo: "Audio information",
tags: {
title: "Tags",
showMore: "Show more",
showLess: "Show less",
obulat marked this conversation as resolved.
Show resolved Hide resolved
},
contentReport: {
short: "Report",
long: "Report this content",
Expand Down
15 changes: 13 additions & 2 deletions frontend/src/types/analytics.ts
Expand Up @@ -283,7 +283,7 @@ export type Events = {
/** Pagination depth */
resultPage: number
}
/*
/**
* Description: Whenever the user clicks the load more button
* Questions:
* - On what page do users typically find a result?
Expand All @@ -301,7 +301,7 @@ export type Events = {
* *before* loading more results.. */
resultPage: number
}
/*
/**
* Description: Whenever the user sets a filter. Filter category and key are the values used in code, not the user-facing filter labels.
* Questions:
* - Do most users filter their searches?
Expand Down Expand Up @@ -412,6 +412,17 @@ export type Events = {
/** the reasons for why this result is considered sensitive */
sensitivities: string
}
/**
* Description: The user expands collapsed tags or collapses the expanded ones.
*
* Questions:
* - Are the extra tags useful to users?
* - Do users ever collapse expanded tags?
*/
TOGGLE_TAG_EXPANSION: {
/** The state of the tags after the user interaction. */
toState: "expanded" | "collapsed"
}
}

/**
Expand Down
6 changes: 4 additions & 2 deletions frontend/src/utils/focus-management.ts
Expand Up @@ -88,8 +88,10 @@ export function isFocusableElement(element: HTMLElement) {
return element.matches(focusableSelector)
}

export function focusElement(element: HTMLElement | null) {
element?.focus()
export function focusElement(element: HTMLElement | Element | null) {
if (element instanceof HTMLElement) {
element.focus()
}
}

// https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/select
Expand Down
5 changes: 5 additions & 0 deletions frontend/test/locales/ar.json
Expand Up @@ -191,6 +191,11 @@
"relatedError": "خطأ في جلب الوسائط ذات الصلة",
"imageInfo": "معلومات الصورة",
"audioInfo": "معلومات صوتية",
"tags": {
"title": "العلامات",
"showMore": "أظهر المزيد",
"showLess": "تظهر أقل"
},
"contentReport": {
"short": "تقرير",
"long": "الإبلاغ عن هذا المحتوى",
Expand Down
47 changes: 46 additions & 1 deletion frontend/test/playwright/e2e/collections.spec.ts
Expand Up @@ -4,14 +4,19 @@
* introduced in https://github.com/WordPress/openverse/pull/3831
*/

import { test, expect } from "@playwright/test"
import { test, expect, Page } from "@playwright/test"

import { preparePageForTests } from "~~/test/playwright/utils/navigation"
import {
getCopyButton,
getH1,
// getLoadMoreButton,
} from "~~/test/playwright/utils/components"
import { t } from "~~/test/playwright/utils/i18n"
import {
collectAnalyticsEvents,
expectEventPayloadToMatch,
} from "~~/test/playwright/utils/analytics"

test.describe.configure({ mode: "parallel" })

Expand Down Expand Up @@ -61,3 +66,43 @@ test.describe("collections", () => {
// expect(await page.locator("figure").count()).toEqual(20)
})
})

const COLLAPSE_BUTTON = (page: Page) =>
page.getByRole("button", { name: t("mediaDetails.tags.showLess") })
const EXPAND_BUTTON = (page: Page) =>
page.getByRole("button", { name: t("mediaDetails.tags.showMore") })

test("some tags are hidden if there are more than 3 rows", async ({ page }) => {
await preparePageForTests(page, "xl", {
features: { additional_search_views: "on" },
})
await page.goto("/image/2bc7dde0-5aad-4cf7-b91d-7f0e3bd06750")

const tags = page.getByRole("list", { name: t("mediaDetails.tags.title") })
await expect(tags).toBeVisible()
const tagsCount = await tags.locator("li").count()

await EXPAND_BUTTON(page).click()
expect(await tags.locator("li").count()).toBeGreaterThan(tagsCount)
})

test("sends analytics events when tags are toggled", async ({
context,
page,
}) => {
await preparePageForTests(page, "xl", {
features: { additional_search_views: "on" },
})
const analyticsEvents = collectAnalyticsEvents(context)
await page.goto("/image/2bc7dde0-5aad-4cf7-b91d-7f0e3bd06750")

await EXPAND_BUTTON(page).click()
await COLLAPSE_BUTTON(page).click()

const toggleEvents = analyticsEvents.filter(
(event) => event.n === "TOGGLE_TAG_EXPANSION"
)
expect(toggleEvents).toHaveLength(2)
expectEventPayloadToMatch(toggleEvents[0], { toState: "expanded" })
expectEventPayloadToMatch(toggleEvents[1], { toState: "collapsed" })
})