Skip to content

Commit

Permalink
Cleanup tag display for long lists of tags (#3808)
Browse files Browse the repository at this point in the history
* Cleanup tag display for long lists of tags

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Fix rtl and extra height

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Add aria-expanded

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Add test locale strings

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Add a constant for number of rows to show

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Update strings

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Add analytics events

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Update the button text

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Use analytics from context

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Simplify focusElement

Signed-off-by: Olga Bulat <obulat@gmail.com>

* Add docs

Signed-off-by: Olga Bulat <obulat@gmail.com>

---------

Signed-off-by: Olga Bulat <obulat@gmail.com>
  • Loading branch information
obulat authored Mar 19, 2024
1 parent 0a4c34a commit 775c0df
Show file tree
Hide file tree
Showing 7 changed files with 534 additions and 21 deletions.
220 changes: 204 additions & 16 deletions frontend/src/components/VMediaInfo/VMediaTags.vue
Original file line number Diff line number Diff line change
@@ -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"
>
<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"
})
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
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,11 @@
},
imageInfo: "Image information",
audioInfo: "Audio information",
tags: {
title: "Tags",
showMore: "Show more",
showLess: "Show less",
},
contentReport: {
short: "Report",
long: "Report this content",
Expand Down
15 changes: 13 additions & 2 deletions frontend/src/types/analytics.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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" })
})
Loading

0 comments on commit 775c0df

Please sign in to comment.