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

Fix VIconButton default variant #1255

Merged
merged 5 commits into from
Apr 22, 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
26 changes: 12 additions & 14 deletions src/components/VAudioTrack/VPlayPause.vue
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
/>
</template>

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

import VIconButton from '~/components/VIconButton/VIconButton.vue'

Expand All @@ -36,22 +36,20 @@ export default defineComponent({
* the current play status of the audio
*/
status: {
type: String,
validator: (val) => ['playing', 'paused', 'played'].includes(val),
type: String as PropType<'playing' | 'paused' | 'played'>,
Copy link
Contributor

Choose a reason for hiding this comment

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

We could use the type from active media store here:

type MediaStatus = 'ejected' | 'playing' | 'paused' // 'ejected' means player is closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a different status, this one has "played" instead of "ejected". IIRC there is some distinction, perhaps one for when the player is closed ("ejected") and the other for when the audio has simply finished playing to the end (in which case we show the replay button).

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, thank you for explaining :)

required: true,
validator: (val: unknown) =>
['playing', 'paused', 'played'].includes(val as string),
},
/**
* The parent audio layout currently in use
* @todo This type def should be extracted for reuse across components
*/
layout: {
type: /** @type {import('@nuxtjs/composition-api').PropType<'full' | 'box' | 'row' | 'global'>} */ (
String
),
type: String as PropType<'full' | 'box' | 'row' | 'global'>,
default: 'full',
/**
* @param {string} val
*/
validator: (val) => ['full', 'box', 'row', 'global'].includes(val),
validator: (val: unknown) =>
['full', 'box', 'row', 'global'].includes(val as string),
},
},
data() {
Expand All @@ -69,19 +67,19 @@ export default defineComponent({
}
},
computed: {
isPlaying() {
isPlaying(): boolean {
return this.status === 'playing'
},
/**
* Get the button label based on the current status of the player.
*/
label() {
label(): string {
return `play-pause.${this.statusVerbMap[this.status]}`
},
/**
* Get the button icon based on the current status of the player.
*/
icon() {
icon(): string {
return this.statusIconMap[this.status]
},
},
Expand Down
6 changes: 6 additions & 0 deletions src/components/VButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import {

import { warn } from '~/utils/console'

import type { ProperlyExtractPropTypes } from '~/types/prop-extraction'

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

const buttonForms = ['VLink', 'button'] as const
Expand Down Expand Up @@ -63,6 +65,10 @@ const buttonTypes = ['button', 'submit', 'reset'] as const

export type ButtonType = typeof buttonTypes[number]

export type ButtonProps = ProperlyExtractPropTypes<
NonNullable<typeof VButton['props']>
>
Comment on lines +68 to +70
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be after the declaration of VButton?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't decide 😅 I thought putting it before would make it more obvious where it was coming from for someone reading this and wondering how it worked 🤔 type declarations and references can appear in any order in a file so it doesn't cause any integrity issues.


/**
* A button component that behaves just like a regular HTML `button` element
* aside from pre-applied styles based on the passed in variant.
Expand Down
8 changes: 6 additions & 2 deletions src/components/VContentReport/VContentReportPopover.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@
<VContentReportButton v-bind="a11yProps" />
</template>
<template #default="{ close }">
<div class="relative">
<div class="relative" data-testid="content-report-popover">
<VIconButton
class="absolute top-0 end-0 border-none"
class="absolute top-0 end-0 border-none text-dark-charcoal-70"
size="search-medium"
:icon-props="{ iconPath: icons.closeSmall }"
:button-props="{
'aria-label': $t('modal.close').toString(),
variant: 'plain',
}"
@click="close"
/>
<VContentReportForm
Expand Down
2 changes: 1 addition & 1 deletion src/components/VFilters/VFilterChecklist.vue
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
<div class="relative">
<VIconButton
:aria-label="$t('modal.close')"
class="absolute top-2 end-2 border-none"
class="absolute top-0 end-0 border-none text-dark-charcoal-70"
size="small"
:icon-props="{ iconPath: icons.closeSmall }"
@click="close"
Expand Down
7 changes: 5 additions & 2 deletions src/components/VIconButton/VIconButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import { computed, defineComponent, PropType } from '@nuxtjs/composition-api'

import VIcon, { IconProps } from '~/components/VIcon/VIcon.vue'
import VButton from '~/components/VButton.vue'
import VButton, { ButtonProps } from '~/components/VButton.vue'
import type { ButtonType } from '~/components/VButton.vue'

const SIZE_MAP = Object.freeze({
Expand Down Expand Up @@ -65,7 +65,10 @@ export default defineComponent({
* props to pass down to the `VButton` component nested inside the button;
* See documentation on `VButton`.
*/
buttonProps: {},
buttonProps: {
type: Object as PropType<ButtonProps>,
default: () => ({ variant: 'plain' }),
},
},
setup(props, { attrs }) {
const type = (attrs['type'] ?? 'button') as ButtonType
Expand Down
4 changes: 2 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-04-18T13:47:09+00:00\n"
"POT-Creation-Date: 2022-04-22T12:44:37+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 @@ -93,7 +93,7 @@ msgctxt "error-images.waiting-for-a-bite"
msgid "Three boys sit on a broken log while two of them fish."
msgstr ""

#: src/components/VFilters/VFilterChecklist.vue:44
#: src/components/VContentReport/VContentReportPopover.vue:18
msgctxt "modal.close"
msgid "Close"
msgstr ""
Expand Down
17 changes: 17 additions & 0 deletions src/types/prop-extraction.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import type {
ExtractPropTypes,
ExtractDefaultPropTypes,
} from '@nuxtjs/composition-api'

/**
* Correctly extracts optional prop types.
*
* I think this is fixed in the Vue 3 types but hasn't been
* ported back to `@vue/composition-api` so for now we have
* to suffer this ugly type.
*/
export type ProperlyExtractPropTypes<P> = Omit<
ExtractPropTypes<P>,
keyof ExtractDefaultPropTypes<P>
> &
Partial<ExtractDefaultPropTypes<P>>
26 changes: 26 additions & 0 deletions test/playwright/visual-regression/content-report-form.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { test, expect } from '@playwright/test'

test.describe('content report form', () => {
test.beforeEach(async ({ page }) => {
await page.goto('/image/feb91b13-422d-46fa-8ef4-cbf1e6ddee9b')
})

test('unfocused close button', async ({ page }) => {
await page.click('button:has-text("report")', { timeout: 500 })
const form = page.locator('[data-testid="content-report-popover"]')

expect(await form.screenshot({ timeout: 500 })).toMatchSnapshot({
name: 'content-report-unfocused.png',
})
})

test('focused close button', async ({ page }) => {
await page.click('button:has-text("report")', { timeout: 500 })
const form = page.locator('[data-testid="content-report-popover"]')

await form.locator('[aria-label="Close"]').focus({ timeout: 500 })
expect(await form.screenshot()).toMatchSnapshot({
name: 'content-report-focused.png',
})
})
})
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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
{
meta: {
createdAt: '2022-04-11T12:31:19.362Z',
host: 'https://api.openverse.engineering',
resHumanReadable: true,
},
req: {
headers: {
accept: 'application/json, text/plain, */*',
connection: 'close',
},
url: '/v1/images/feb91b13-422d-46fa-8ef4-cbf1e6ddee9b/',
method: 'GET',
body: '',
},
res: {
status: 200,
headers: {
date: [
'Mon, 11 Apr 2022 12:31:19 GMT',
],
'content-type': [
'application/json',
],
'transfer-encoding': [
'chunked',
],
connection: [
'close',
],
vary: [
'Accept-Encoding, Accept, Authorization, Origin',
],
allow: [
'GET, HEAD, OPTIONS',
],
'x-frame-options': [
'DENY',
],
'x-content-type-options': [
'nosniff',
],
'referrer-policy': [
'same-origin',
],
'cross-origin-opener-policy': [
'same-origin',
],
'cache-control': [
'max-age=14400',
],
'cf-cache-status': [
'HIT',
],
age: [
'533',
],
'last-modified': [
'Mon, 11 Apr 2022 12:22:26 GMT',
],
'expect-ct': [
'max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"',
],
'strict-transport-security': [
'max-age=15552000; includeSubDomains; preload',
],
server: [
'cloudflare',
],
'cf-ray': [
'6fa3bd128beb0e12-MXP',
],
'alt-svc': [
'h3=":443"; ma=86400, h3-29=":443"; ma=86400',
],
},
body: {
id: 'feb91b13-422d-46fa-8ef4-cbf1e6ddee9b',
title: 'Galah in Darwin (Eolophus roseicapilla)',
foreign_landing_url: 'https://www.flickr.com/photos/126953422@N04/40593461235',
creator: 'Graham Winterflood',
creator_url: 'https://www.flickr.com/photos/126953422@N04',
url: 'https://live.staticflickr.com/894/40593461235_b72593df28_b.jpg',
filesize: null,
filetype: null,
license: 'by-sa',
license_version: '2.0',
license_url: 'https://creativecommons.org/licenses/by-sa/2.0/',
provider: 'flickr',
source: 'flickr',
category: null,
tags: [
{
name: 'bird',
},
{
name: 'canoneos7d',
},
{
name: 'eolophusroseicapilla',
},
{
name: 'galah',
},
],
attribution: '"Galah in Darwin (Eolophus roseicapilla)" by Graham Winterflood is licensed under CC-BY-SA 2.0. To view a copy of this license, visit https://creativecommons.org/licenses/by-sa/2.0/.',
height: 683,
width: 1024,
thumbnail: 'http://localhost:49153/v1/images/feb91b13-422d-46fa-8ef4-cbf1e6ddee9b/thumb/',
detail_url: 'http://localhost:49153/v1/images/feb91b13-422d-46fa-8ef4-cbf1e6ddee9b/',
related_url: 'http://localhost:49153/v1/images/feb91b13-422d-46fa-8ef4-cbf1e6ddee9b/related/',
},
},
}
Loading