This repository has been archived by the owner on Feb 22, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 63
Add VContentLink
component
#560
Merged
+278
−9
Merged
Changes from 29 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
3855ed7
Add VHeader stub
obulat d9bd5fa
Extract search route handling into composable, set Logo loading when …
obulat 2411a8e
Update pnpm lockfile
zackkrida 0291140
Merge branch 'redesign' into add/VHeader
zackkrida 8a99444
Add VFilterButton (#489)
obulat 6413ac4
Merge redesign into header
zackkrida c9b812f
merge main
zackkrida 925e9bb
merge base branch
zackkrida 97ad095
Add searchbar to header (#491)
obulat 7b19dfa
Add `VContentLink` component
500a673
Set currentColor on icons
d3baf5c
Add hover and focus styles
4f73028
Add Storybook story
85a1dc2
Translate mediaType label
dc59f1f
Add Storybook story for mobile view
f6d98ce
Allow mediaType selection on mobile story
9f3407c
Add unit tests for `VContentLink`
75554bf
Update references to `VInputField` in story
ebcfabe
Change label of image tab to plural
b8d2af1
Add desktop version
ce40ddd
Merge branch 'redesign' into add/VHeader
zackkrida e1d040d
Extract function for resultsCount i18n into composable
729b4d8
Update references to `InputField` to point to `VInputField` (#586)
dhruvkb 047b2e5
New 404 page (#583)
zackkrida dfafab1
Merge branch 'add/VHeader' into v-content-link
6ce5580
Merge branch 'redesign' into v-content-link
8d95967
Update e2e tests
16defda
Double default timeout of playwright tests
682cd06
merge redesign
zackkrida dbe772a
Add truthy check for aria attribute
33f21f2
Pass `$listeners` instead of using custom event
30256de
Use border transparent on focus
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,5 +4,6 @@ const config = { | |
use: { | ||
baseURL: 'http://localhost:8443', | ||
}, | ||
timeout: 1 * 60 * 1000, | ||
} | ||
module.exports = config |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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: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" | ||
@click="$emit('selected', mediaType)" | ||
> | ||
<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), | ||
}, | ||
}, | ||
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> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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" | ||
@selected="onSelected"/> | ||
</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' }, | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 }) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
import VContentLink from '~/components/VContentLink/VContentLink.vue' | ||
import { render, screen } from '@testing-library/vue' | ||
import userEvent from '@testing-library/user-event' | ||
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
|
||
}) | ||
|
||
it('should emit selected event on click', () => { | ||
const { emitted } = render(VContentLink, options) | ||
const btn = screen.getByText('search-tab.image') | ||
userEvent.click(btn) | ||
expect(emitted().selected).toBeTruthy() | ||
}) | ||
krysal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.