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

Commit

Permalink
Fix back to search link for pages in other languages (#1350)
Browse files Browse the repository at this point in the history
* Fix back to search link for pages in other languages

* Replace inline svg component with VIcon

* Add back to search link e2e test for localized pages (#1351)

* Update src/pages/audio/_id.vue

Co-authored-by: Krystle Salazar <krystle.salazar@automattic.com>

* Remove unnecessary showBackToSearch data property

* Add POT file

* Update POT file

Co-authored-by: Krystle Salazar <krystle.salazar@automattic.com>
  • Loading branch information
obulat and krysal authored May 3, 2022
1 parent ca08f8a commit a0db758
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 95 deletions.
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 @@ -455,7 +455,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 @@ -1120,7 +1120,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
export default SVG
}

Expand Down

0 comments on commit a0db758

Please sign in to comment.