Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Fix back to search link for pages in other languages #1350

Merged
merged 8 commits into from
May 3, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 13 additions & 21 deletions src/components/VBackToSearchResultsLink.vue
Original file line number Diff line number Diff line change
@@ -1,43 +1,35 @@
<template>
<!-- @todo: Separate the absolute container from the link itself. -->
<VLink
v-if="show"
class="px-2 pt-1 md:px-6 md:pt-4 md:pb-2 flex flex-row items-center font-semibold text-dark-charcoal text-xs md:text-sr"
:href="path"
>
<Chevron class="-ms-2" />
<VIcon :icon-path="chevronIcon" class="-ms-2" />
{{ $t('single-result.back') }}
</VLink>
</template>

<script>
import { defineComponent } from '@vue/composition-api'
<script lang="ts">
import { defineComponent } from '@nuxtjs/composition-api'

import VIcon from '~/components/VIcon/VIcon.vue'
import VLink from '~/components/VLink.vue'

import Chevron from '~/assets/icons/chevron-left.svg?inline'
import chevronIcon from '~/assets/icons/chevron-left.svg'

export default defineComponent({
components: {
Chevron,
VIcon,
VLink,
},
data() {
return {
/** @type {undefined|string} */
path: undefined,
show: false,
}
props: {
path: {
type: String,
required: true,
},
},
created() {
if (!this.$nuxt?.context?.from?.fullPath) {
return
}

this.path = this.$nuxt.context.from.fullPath
if (this.path.startsWith('/search')) {
this.show = true
}
setup() {
return { chevronIcon }
},
})
</script>
6 changes: 3 additions & 3 deletions src/locales/po-files/openverse.pot
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ msgid ""
msgstr ""
"Project-Id-Version: Openverse \n"
"Report-Msgid-Bugs-To: https://github.com/wordpress/openverse/issues \n"
"POT-Creation-Date: 2022-05-03T05:26:54+00:00\n"
"POT-Creation-Date: 2022-05-03T05:46:51+00:00\n"
"MIME-Version: 1.0\n"
"Content-Type: text/plain; charset=UTF-8\n"
"Content-Transfer-Encoding: 8bit\n"
Expand Down Expand Up @@ -418,7 +418,7 @@ msgctxt "image-details.information.unknown"
msgid "Unknown"
msgstr ""

#: src/components/VBackToSearchResultsLink.vue:9
#: src/components/VBackToSearchResultsLink.vue:8
msgctxt "single-result.back"
msgid "Back to search results"
msgstr ""
Expand Down Expand Up @@ -1083,7 +1083,7 @@ msgid "An error occurred"
msgstr ""

#. Do not translate words between ### ###.
#: src/pages/image/_id.vue:144
#: src/pages/image/_id.vue:142
msgctxt "error.image-not-found"
msgid "Couldn't find image with id ###id###"
msgstr ""
Expand Down
8 changes: 4 additions & 4 deletions src/pages/audio/_id.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<template>
<main class="relative">
<div class="w-full p-2">
<VBackToSearchResultsLink />
<div v-if="backToSearchPath" class="w-full p-2">
<VBackToSearchResultsLink :path="backToSearchPath" />
</div>
<VAudioTrack layout="full" :audio="audio" class="main-track" />
<div
Expand Down Expand Up @@ -43,7 +43,7 @@ const AudioDetailPage = {
},
data() {
return {
showBackToSearchLink: false,
backToSearchPath: '',
}
},
setup() {
Expand Down Expand Up @@ -81,7 +81,7 @@ const AudioDetailPage = {
from.name === _this.localeRoute({ path: '/search/' }).name ||
from.name === _this.localeRoute({ path: '/search/audio' }).name
) {
_this.showBackToSearchLink = true
_this.backToSearchPath = from.fullPath
}
})
},
Expand Down
12 changes: 5 additions & 7 deletions src/pages/image/_id.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
<div>
<figure class="w-full mb-4 pt-8 md:pt-12 px-6 bg-dark-charcoal-06 relative">
<div
v-if="showBackToSearchLink"
v-if="backToSearchPath"
class="absolute left-0 top-0 right-0 z-40 w-full px-2"
>
<VBackToSearchResultsLink />
<VBackToSearchResultsLink :path="backToSearchPath" />
</div>

<img
Expand Down Expand Up @@ -83,7 +83,6 @@ import { useSingleResultStore } from '~/stores/media/single-result'
import { useRelatedMediaStore } from '~/stores/media/related-media'

import VButton from '~/components/VButton.vue'
import VIcon from '~/components/VIcon/VIcon.vue'
import VLink from '~/components/VLink.vue'
import VImageDetails from '~/components/VImageDetails/VImageDetails.vue'
import VMediaReuse from '~/components/VMediaInfo/VMediaReuse.vue'
Expand All @@ -95,7 +94,6 @@ const VImageDetailsPage = {
name: 'VImageDetailsPage',
components: {
VButton,
VIcon,
VLink,
VImageDetails,
VMediaReuse,
Expand All @@ -109,7 +107,7 @@ const VImageDetailsPage = {
imageHeight: 0,
imageType: 'Unknown',
isLoadingFullImage: true,
showBackToSearchLink: false,
backToSearchPath: '',
sketchFabfailure: false,
}
},
Expand Down Expand Up @@ -156,7 +154,7 @@ const VImageDetailsPage = {
from.name === _this.localeRoute({ path: '/search/' }).name ||
from.name === _this.localeRoute({ path: '/search/image' }).name
) {
_this.showBackToSearchLink = true
_this.backToSearchPath = from.fullPath
}
})
},
Expand All @@ -174,7 +172,7 @@ const VImageDetailsPage = {
})
.catch(() => {
/**
* Do nothing. This avoid the console warning "Uncaught (in promise) Error:
* Do nothing. This avoids the console warning "Uncaught (in promise) Error:
* Network Error" in Firefox in development mode.
*/
})
Expand Down
47 changes: 47 additions & 0 deletions test/playwright/e2e/search-navigation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,50 @@ test.describe('search history navigation', () => {
expect(await page.locator('text=See all audio').isVisible()).toBe(true)
})
})

test('navigates to the image detail page correctly', async ({ page }) => {
await page.goto('/search/image?q=honey')
const figure = page.locator('figure').first()
const imgTitle = await figure.locator('img').getAttribute('alt')

await page.locator('a[href^="/image"]').first().click()
// Until the image is loaded, the heading is 'Image' instead of the actual title
await page.locator('#main-image').waitFor()

const headingText = await page.locator('h1').textContent()
expect(headingText?.trim().toLowerCase()).toEqual(imgTitle?.toLowerCase())
})

test.describe('back to search results link', () => {
test('is visible in breadcrumb when navigating to image details page and returns to the search page', async ({
page,
}) => {
const url = '/search/?q=galah'
await page.goto(url)
await page.locator('a[href^="/image"]').first().click()
const link = page.locator('text="Back to search results"')
await expect(link).toBeVisible()
await link.click()
await expect(page).toHaveURL(url)
})

test('is visible in breadcrumb when navigating to localized image details page', async ({
page,
}) => {
await page.goto('/es/search/?q=galah')
await page.locator('a[href^="/es/image"]').first().click()
await expect(
page.locator('text="Volver a los resultados de búsqueda"')
).toBeVisible()
})

test('is visible in breadcrumb when navigating to localized audio details page', async ({
page,
}) => {
await page.goto('/es/search/?q=galah')
await page.locator('a[href^="/es/audio"]').first().click()
await expect(
page.locator('text="Volver a los resultados de búsqueda"')
).toBeVisible()
})
})
51 changes: 0 additions & 51 deletions test/playwright/e2e/search.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,58 +14,7 @@ test.beforeEach(async ({ context }) => {
await mockProviderApis(context)
})

test.skip('shows search result metadata', async ({ page }) => {
await page.goto('/search/image?q=cat&source=rijksmuseum')
await page.route('https://api.openverse.engineering/v1/images/**', (route) =>
route.fulfill({ path: 'test/e2e/resources/last_page.json' })
)

// Expect results meta
// Flaky test. Is there a way to mock the first api result when loading on the server?
await expect(page.locator('.results-meta span.pe-6')).toContainText(
/image results/
)
await expect(page.locator('text=Are these results relevant?')).toHaveCount(1)

// Click load more button
const loadMoreButton = page.locator('button:has-text("Load more results")')
await expect(loadMoreButton).toHaveCount(1)
await loadMoreButton.click()

// Click load more button
await expect(loadMoreButton).toHaveCount(1)
await loadMoreButton.click()
// All search results have been shown, cannot load more
await expect(loadMoreButton).toHaveCount(0, { timeout: 300 })
})

test('shows no results page when no results', async ({ page }) => {
await page.goto('/search/image?q=243f6a8885a308d3')
await expect(page.locator('.error-section')).toBeVisible()
})

test('navigates to the image detail page correctly', async ({ page }) => {
await page.goto('/search/image?q=honey')
const figure = page.locator('figure').first()
const imgTitle = await figure.locator('img').getAttribute('alt')

await page.locator('a[href^="/image"]').first().click()
// Until the image is loaded, the heading is 'Image' instead of the actual title
await page.locator('#main-image').waitFor()

const headingText = await page.locator('h1').textContent()
expect(headingText?.trim().toLowerCase()).toEqual(imgTitle?.toLowerCase())
// Renders the breadcrumb link
await expect(page.locator('text="Back to search results"')).toBeVisible()
})

test('the Back to search results link returns to the search page', async ({
page,
}) => {
const url = '/search/image?q=honey'
await page.goto(url)

await page.locator('a[href^="/image"]').first().click()
await page.locator('text="Back to search results"').click()
await expect(page).toHaveURL(url)
})
14 changes: 7 additions & 7 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,24 @@
"src/components/VWarningSuppressor.vue",
"src/components/VLink.vue",
"src/components/VLoadMore.vue",
"src/components/VTranslationStatusBanner.vue",
"src/components/VNotificationBanner.vue",
"src/components/VMigrationNotice.vue",
"src/components/VVariations.vue",
"src/components/VBackToSearchResultsLink.vue",
"src/components/VSourcesTable.vue",
"src/components/TableSortIcon.vue",
"src/components/VIcon/VIcon.vue",
"src/components/VIconButton/VIconButton.vue",
"src/components/VLicense/VLicense.vue",
"src/components/VContentReport/VReportDescForm.vue",
"src/components/VErrorSection/VErrorImage.vue",
"src/components/VTranslationStatusBanner.vue",
"src/components/VNotificationBanner.vue",
"src/components/VMigrationNotice.vue",
"src/components/VVariations.vue",
"src/components/VAudioTrack/**.vue",
"src/components/VAudioThumbnail/VAudioThumbnail.vue",
"src/components/VGlobalAudioSection/VGlobalAudioSection.vue",
"src/components/VInputField/VInputField.vue",
"src/components/VHeader/VFilterButton.vue",
"src/components/VHeader/VSearchBar/VSearchBar.vue",
"src/components/VAudioTrack/VPlayPause.vue",
"src/components/VSourcesTable.vue",
"src/components/TableSortIcon.vue",
"src/components/VTabs/**.vue",
"src/components/VCopyButton.vue",
"src/components/VMediaInfo/**.vue",
Expand Down
4 changes: 2 additions & 2 deletions typings/openverse/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ declare module '*.svg' {
export default SVG
}

declare module '*.svg!inline' {
const SVG: string
declare module '*.svg?inline' {
const SVG: unknown
Comment on lines +6 to +7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice that at least this removes the warning on files using these imports :)

export default SVG
}

Expand Down