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

Add VContentLink component #560

Merged
merged 32 commits into from
Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
3855ed7
Add VHeader stub
obulat Dec 6, 2021
d9bd5fa
Extract search route handling into composable, set Logo loading when …
obulat Dec 14, 2021
2411a8e
Update pnpm lockfile
zackkrida Dec 15, 2021
0291140
Merge branch 'redesign' into add/VHeader
zackkrida Dec 15, 2021
8a99444
Add VFilterButton (#489)
obulat Dec 16, 2021
6413ac4
Merge redesign into header
zackkrida Dec 16, 2021
c9b812f
merge main
zackkrida Jan 3, 2022
925e9bb
merge base branch
zackkrida Jan 5, 2022
97ad095
Add searchbar to header (#491)
obulat Jan 6, 2022
7b19dfa
Add `VContentLink` component
Dec 23, 2021
500a673
Set currentColor on icons
Jan 3, 2022
d3baf5c
Add hover and focus styles
Jan 4, 2022
4f73028
Add Storybook story
Jan 4, 2022
85a1dc2
Translate mediaType label
Jan 4, 2022
dc59f1f
Add Storybook story for mobile view
Jan 4, 2022
f6d98ce
Allow mediaType selection on mobile story
Jan 5, 2022
9f3407c
Add unit tests for `VContentLink`
Jan 5, 2022
75554bf
Update references to `VInputField` in story
Jan 6, 2022
ebcfabe
Change label of image tab to plural
Jan 6, 2022
b8d2af1
Add desktop version
Jan 6, 2022
ce40ddd
Merge branch 'redesign' into add/VHeader
zackkrida Jan 7, 2022
e1d040d
Extract function for resultsCount i18n into composable
Jan 7, 2022
729b4d8
Update references to `InputField` to point to `VInputField` (#586)
dhruvkb Jan 7, 2022
047b2e5
New 404 page (#583)
zackkrida Jan 10, 2022
dfafab1
Merge branch 'add/VHeader' into v-content-link
Jan 10, 2022
6ce5580
Merge branch 'redesign' into v-content-link
Jan 12, 2022
8d95967
Update e2e tests
Jan 12, 2022
16defda
Double default timeout of playwright tests
Jan 12, 2022
682cd06
merge redesign
zackkrida Jan 14, 2022
dbe772a
Add truthy check for aria attribute
Jan 14, 2022
33f21f2
Pass `$listeners` instead of using custom event
Jan 14, 2022
30256de
Use border transparent on focus
Jan 14, 2022
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
1 change: 1 addition & 0 deletions playwright.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ const config = {
use: {
baseURL: 'http://localhost:8443',
},
timeout: 1 * 60 * 1000,
}
module.exports = config
9 changes: 9 additions & 0 deletions src/assets/icons/audio-wave.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions src/assets/icons/image.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
95 changes: 95 additions & 0 deletions src/components/VContentLink/VContentLink.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
<template>
<button
class="bg-white border border-dark-charcoal/20 rounded-sm flex hover:bg-dark-charcoal hover:text-white focus:bg-white focus:border-tx focus:ring focus:ring-pink focus:outline-none focus:shadow-ring focus:text-black overflow-hidden"
:class="[
isStacked
? 'flex-col items-start py-4 ps-4 pe-12'
: 'flex-row justify-between items-center w-full min-w-[22rem] p-6',
]"
role="radio"
type="button"
:aria-checked="isSelected"
v-on="$listeners"
>
<div
class="flex"
:class="[isStacked ? 'flex-col items-start' : ' flex-row items-center']"
>
<VIcon :icon-path="iconPath" />
<p class="font-semibold" :class="[isStacked ? 'pt-1' : 'ps-2 text-2xl']">
{{
isStacked
? $t(`search-tab.${mediaType}`)
: $t(`search-tab.see-${mediaType}`)
}}
</p>
</div>
<span :class="{ 'text-sr': !isStacked }">{{ resultsCountLabel }}</span>
</button>
</template>

<script>
import { computed, defineComponent } from '@nuxtjs/composition-api'
import { resultsCount } from '~/composables/use-i18n-utilities'
import { AUDIO, IMAGE, mediaTypes } from '~/constants/media'
import VIcon from '~/components/VIcon/VIcon.vue'

import audioIcon from '~/assets/icons/audio-wave.svg'
import imageIcon from '~/assets/icons/image.svg'

export default defineComponent({
name: 'VContentLink',
components: { VIcon },
props: {
/**
* One of the media types supported.
*/
mediaType: {
type: String,
required: true,
validator: (val) => mediaTypes.includes(val),
},
/**
* The number of results that the search returned.
*/
resultsCount: {
type: Number,
required: true,
},
/**
* Whether the indicated media type is currently selected.
*/
isSelected: {
type: Boolean,
default: false,
},
/**
* `stacked` intended for mobile and `horizontal` for desktop.
*/
layout: {
type: String,
default: 'stacked',
validator: (val) => ['stacked', 'horizontal'].includes(val),
},
Comment on lines +69 to +73
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a shame we have changes other than just organization that get tied to this (like font size I think?) because it seems like something that flex-wrap should be able to take care of.

Do we envision the usage of this prop to be a situation where the component is rendered in distinct places where the user will know whether it's a mobile or desktop context? Or are we potentially just pushing the media-query up the stack so that the code for this prop will be something like like :layout="isMinScreen('md') ? 'horizontal' : 'stacked'"?

If it's the last case then my inclination would be to just move that logic into the component and hide the fact that it has different layouts depending on screen size from the user (so the user doesn't have to worry about it).

This is not a blocker or even a solid suggestion, just wondering what the intention is.

Copy link
Member

Choose a reason for hiding this comment

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

I also started thinking we might be able to handle this with just css-based media queries. I can look at that when I implement this component in #530 and my 'all' results PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Using the isMinScreen() is giving me mismatched HTML problems when the component is server-side rendered. I'll have to take a closer look at how those errors are being handled but for now I'd like to tie this together. I agree that it would be ideal to hide this in the component logic and save us a property here as well.

},
setup(props) {
const iconMapping = {
[AUDIO]: audioIcon,
[IMAGE]: imageIcon,
}
const iconPath = computed(() => iconMapping[props.mediaType])

const resultsCountLabel = computed(() => resultsCount(props.resultsCount))

const isStacked = computed(() => props.layout == 'stacked')

return { iconPath, imageIcon, resultsCountLabel, isStacked }
},
})
</script>

<style scoped>
button[aria-checked='true'] {
@apply bg-dark-charcoal text-white;
}
</style>
83 changes: 83 additions & 0 deletions src/components/VContentLink/meta/VContentLink.stories.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import VContentLink from '~/components/VContentLink/VContentLink.vue'

export default {
title: 'Components/VContentLink',
component: VContentLink,
argTypes: {
mediaType: {
options: ['audio', 'image'],
control: { type: 'radio' },
},
resultsCount: {
control: { type: 'number' },
},
isSelected: {
control: { type: 'boolean' },
},
layout: {
options: ['stacked', 'horizontal'],
control: { type: 'radio' },
},
},
}

const VContentLinkStory = (args, { argTypes }) => ({
template: `<VContentLink v-bind="$props" />`,
components: { VContentLink },
props: Object.keys(argTypes),
})

const TwoVContentLinkStory = () => ({
template: `
<div class="grid grid-cols-2 gap-4">
<VContentLink
v-for="(type, key) in types"
:key="key"
:media-type="type.mediaType"
:results-count="type.resultsCount"
:is-selected="selected == type.mediaType"
@click="onSelected(type.mediaType)"/>
</div>
`,
components: { VContentLink },
data() {
return {
types: [
{ mediaType: 'image', resultsCount: 4321 },
{ mediaType: 'audio', resultsCount: 1234 },
],
selected: 'audio',
}
},
methods: {
onSelected(mediaType) {
this.selected = mediaType
},
},
})

export const Default = VContentLinkStory.bind({})
Default.args = {
mediaType: 'image',
resultsCount: 5708,
isSelected: false,
}

const VContentLinkHorizontalStory = (args, { argTypes }) => ({
template: `<div class="max-w-md"><VContentLink v-bind="$props" /></div>`,
components: { VContentLink },
props: Object.keys(argTypes),
})

export const Horizontal = VContentLinkHorizontalStory.bind({})
Horizontal.args = {
mediaType: 'audio',
resultsCount: 4561,
isSelected: false,
layout: 'horizontal',
}

export const Mobile = TwoVContentLinkStory.bind({})
Mobile.parameters = {
viewport: { defaultViewport: 'mob' },
}
4 changes: 2 additions & 2 deletions src/components/VInputField/meta/VInputField.stories.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ export const Template = (args, { argTypes }) => ({

# Input field

<Description of={InputField} />
<Description of={VInputField} />

<ArgsTable of={InputField} />
<ArgsTable of={VInputField} />

The component emits an `input` event with the new contents of the field whenever
the field receives an input.
Expand Down
24 changes: 24 additions & 0 deletions src/composables/use-i18n-utilities.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { useContext } from '@nuxtjs/composition-api'
import { ALL_MEDIA } from '~/constants/media'

/**
* Returns the localized text for the number of search results according to the
* media type.
*
* @param {number} resultsCount
* @param {string} mediaType
* @returns {string}
*/
export function resultsCount(resultsCount, mediaType = ALL_MEDIA) {
const { i18n } = useContext()

const countKey =
resultsCount === 0
? 'no-results'
: resultsCount >= 10000
? 'result-count-more'
: 'result-count'
const fullKey = `browse-page.${mediaType}-${countKey}`
const localeCount = resultsCount.toLocaleString(i18n.locale)
return i18n.tc(fullKey, resultsCount, { localeCount })
}
6 changes: 4 additions & 2 deletions src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,11 @@
},
"search-tab": {
"all": "All",
"image": "Image",
"image": "Images",
"audio": "Audio",
"video": "Video"
"video": "Video",
"see-image": "See all images",
"see-audio": "See all audio"
},
"filters": {
"title": "Filters",
Expand Down
12 changes: 10 additions & 2 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-01-11T19:17:21+00:00\n"
"POT-Creation-Date: 2022-01-12T20:51:27+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 @@ -1112,7 +1112,7 @@ msgid "All"
msgstr ""

msgctxt "search-tab.image"
msgid "Image"
msgid "Images"
msgstr ""

msgctxt "search-tab.audio"
Expand All @@ -1123,6 +1123,14 @@ msgctxt "search-tab.video"
msgid "Video"
msgstr ""

msgctxt "search-tab.see-image"
msgid "See all images"
msgstr ""

msgctxt "search-tab.see-audio"
msgid "See all audio"
msgstr ""

msgctxt "error.occurred"
msgid "An error occurred"
msgstr ""
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/content-types.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const contentTypes = [
},
{
id: 'image',
name: 'Image',
name: 'Images',
url: '/search/image/?q=cat',
supported: true,
sources: 6,
Expand All @@ -63,7 +63,7 @@ const contentTypes = [
]

for (const [i, contentType] of contentTypes.entries()) {
test(`Can open ${contentType.name} page on SSR`, async ({ page }) => {
test(`Can open ${contentType.name} search page on SSR`, async ({ page }) => {
await page.goto(contentType.url)

// Selected content page
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/search-query-client.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ test('selecting license type filter on homepage applies filters', async ({
const activeTabLabel = await page
.locator('[role="tab"][aria-selected="true"]')
.textContent()
expect(activeTabLabel.trim()).toEqual('Image')
expect(activeTabLabel.trim()).toEqual('Images')
await expect(page).toHaveURL('search/image?q=cat&license_type=commercial')
})

Expand Down
41 changes: 41 additions & 0 deletions test/unit/specs/components/v-content-link.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import VContentLink from '~/components/VContentLink/VContentLink.vue'
import { render, screen } from '@testing-library/vue'
import VueI18n from 'vue-i18n'

const enMessages = require('~/locales/en.json')
const i18n = new VueI18n({
locale: 'en',
fallbackLocale: 'en',
messages: { en: enMessages },
})

describe('VContentLink', () => {
let options = {}

beforeEach(() => {
options = {
props: { mediaType: 'image', resultsCount: 123 },
mocks: {
$nuxt: {
context: {
i18n,
},
},
},
}
})

it('is not selected by default', () => {
render(VContentLink, options)
const btn = screen.getByRole('radio')
expect(btn).not.toHaveAttribute('aria-checked')
})

it('is marked as selected when indicated with the isSelected prop', () => {
options.props.isSelected = true
render(VContentLink, options)
const btn = screen.getByRole('radio')
expect(btn).toHaveAttribute('aria-checked')
krysal marked this conversation as resolved.
Show resolved Hide resolved
expect(btn.getAttribute('aria-checked')).toBeTruthy()
})
})