Skip to content
Open
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
58 changes: 56 additions & 2 deletions resources/js/components/fieldtypes/VideoFieldtype.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
<template>
<div class="flex flex-col space-y-3 p-1.5 bg-gray-100 border border-gray-300 dark:bg-gray-900 dark:border-gray-700 rounded-xl">
<ui-input-group>
<ui-combobox
:model-value="mode"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The description says "the stored value is the single source of truth for which input is shown, so the two can't disagree" — but they can, because which input renders is driven by isCloudflare (value-derived) while which option the dropdown displays is bound to mode, which is seeded once from meta.video.provider (server-derived).

Those disagree whenever the stored cloudflare: value is malformed: Embed::fromValue() reports provider unsupported for it, while the JS only checks the prefix. Mounting with value: 'cloudflare:ABC-123' and meta.video.provider: 'unsupported' gives:

  • isCloudflaretrue, so the ID input renders showing ABC-123
  • mode'url', so the dropdown reads URL

This is reachable — isInvalid is only a visual hint and there's no server-side validation, so a malformed ID saves fine and the field comes back in that contradictory state on reload.

Binding to the same source of truth fixes it in every case, since isCloudflare already falls back to mode when there's no value:

Suggested change
:model-value="mode"
:model-value="isCloudflare ? 'cloudflare' : 'url'"

With that, mode goes back to being purely the empty-field seed, which is what the comment on it already claims.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The description says "the stored value is the single source of truth for which input is shown, so the two can't disagree" — but they can. Which input renders is driven by isCloudflare, which only checks the prefix. Which option the dropdown displays is bound to mode, seeded once from meta.video.provider — and Embed::fromValue() reports unsupported for a malformed ID rather than cloudflare.

So they disagree whenever the stored value is a malformed cloudflare:. Mounting with value: 'cloudflare:ABC-123' and meta.video.provider: 'unsupported':

  • isCloudflaretrue, so the ID input renders showing ABC-123
  • mode'url', so the dropdown reads URL

This is reachable — isInvalid is only a visual hint and the fieldtype declares no validation rules, so a malformed ID saves fine and the field comes back in that contradictory state on reload.

Binding to the same source of truth fixes it in every case, since isCloudflare already falls back to mode when there's no value:

Suggested change
:model-value="mode"
:model-value="isCloudflare ? 'cloudflare' : 'url'"

That also leaves mode as purely the empty-field seed, which is what the comment on it already claims it is.

:options="meta.providers"
option-label="label"
option-value="value"
:aria-label="__('Video Provider')"
@update:model-value="changeMode"
/>
Comment on lines +5 to +10

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The combobox needs isReadOnly passed through. Both ui-inputs get it, this doesn't, and ui-combobox supports a readOnly prop (resources/js/components/ui/Combobox/Combobox.vue:61) — so nothing stops the user changing the provider on a read-only field, and changeMode() then calls this.update(null).

I confirmed it by mounting this component with config: { visibility: 'read_only' } and a populated YouTube value: isReadOnly is true, and selecting Cloudflare emits update:value[null]. The field is now dirty and saving persists the wipe. Same applies to visibility: computed fields.

Suggested change
:options="meta.providers"
option-label="label"
option-value="value"
:aria-label="__('Video Provider')"
@update:model-value="changeMode"
/>
:options="meta.providers"
option-label="label"
option-value="value"
:read-only="isReadOnly"
:aria-label="__('Video Provider')"
@update:model-value="changeMode"
/>

Comment on lines +5 to +10

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The combobox needs isReadOnly passed through. Both ui-inputs get it, this doesn't, and ui-combobox supports a readOnly prop (resources/js/components/ui/Combobox/Combobox.vue:61) — so nothing stops the user changing the provider on a read-only field, and changeMode() then calls this.update(null).

Confirmed by mounting this component with config: { visibility: 'read_only' } and value: 'https://vimeo.com/1': isReadOnly is true, and selecting Cloudflare emits update:value[null]. The field is now dirty and saving persists the wipe. Same applies to visibility: computed.

Suggested change
:options="meta.providers"
option-label="label"
option-value="value"
:aria-label="__('Video Provider')"
@update:model-value="changeMode"
/>
:options="meta.providers"
option-label="label"
option-value="value"
:read-only="isReadOnly"
:aria-label="__('Video Provider')"
@update:model-value="changeMode"
/>

<ui-input-group v-if="isCloudflare">
<ui-input-group-prepend :text="__('ID')" />
<ui-input
:model-value="videoId"
:isReadOnly="isReadOnly"
:aria-label="__('Video ID')"
@update:model-value="updateCloudflareId"
@focus="$emit('focus')"
@blur="$emit('blur')"
input-class="border-s-0"
/>
</ui-input-group>
<ui-input-group v-else>
<ui-input-group-prepend :text="__('URL')" />
<ui-input
:model-value="value"
Expand Down Expand Up @@ -29,22 +49,32 @@
<script>
import Fieldtype from './Fieldtype.vue';

const CLOUDFLARE = 'cloudflare';
const CLOUDFLARE_PREFIX = 'cloudflare:';
const URL_MODE = 'url';

export default {
mixins: [Fieldtype],

data() {
return {
isVisible: false,
observer: null,
// Only consulted when there's no value; otherwise the value itself says which input to show.
mode: this.meta.video?.provider === CLOUDFLARE ? CLOUDFLARE : URL_MODE,
};
},

computed: {
shouldShowPreview() {
return !this.isInvalid && (this.isEmbeddable || this.isVideo);
return !this.isInvalid && (this.isCloudflare ? !!this.videoId : this.isEmbeddable || this.isVideo);
},

embedUrl() {
if (this.isCloudflare) {
return this.videoId ? `https://iframe.cloudflarestream.com/${this.videoId}` : null;
}

let embed_url = this.value || '';

if (embed_url.includes('youtube')) {
Expand Down Expand Up @@ -73,6 +103,10 @@ export default {
return embed_url;
},

isCloudflare() {
return this.value?.startsWith(CLOUDFLARE_PREFIX) || (!this.value && this.mode === CLOUDFLARE);
},

isEmbeddable() {
const url = this.value || '';
const isYoutube = url.includes('youtube') || url.includes('youtu.be');
Expand All @@ -81,6 +115,8 @@ export default {
},

isInvalid() {
if (this.isCloudflare) return !!this.videoId && !/^[a-zA-Z0-9]+$/.test(this.videoId);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor, but when this branch fails the message rendered below is statamic::validation.url (line 36), which is "Must be a valid URL." That shows up under the ID input and tells the user to enter a valid URL in a field that isn't one. Worth a dedicated string.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor, but when this branch returns true the message rendered below is statamic::validation.url (line 36) — "Must be a valid URL." That shows up under the ID input and tells the user to enter a valid URL in a field that isn't one. Worth a dedicated string.


let htmlRegex = new RegExp(/<([A-Z][A-Z0-9]*)\b[^>]*>.*?<\/\1>|<([A-Z][A-Z0-9]*)\b[^\/]*\/>/i);
return htmlRegex.test(this.value || '');
},
Expand All @@ -95,6 +131,24 @@ export default {
const isVideo = url.includes('.mp4') || url.includes('.ogv') || url.includes('.mov') || url.includes('.webm');
return !this.isEmbeddable && isVideo;
},

videoId() {
return this.value?.startsWith(CLOUDFLARE_PREFIX) ? this.value.slice(CLOUDFLARE_PREFIX.length) : null;
},
},

methods: {
changeMode(mode) {
if (mode === this.mode) return;

this.mode = mode;

if (this.value) this.update(null);
},
Comment on lines +141 to +147

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

changeMode races the pending debounced update and loses, because the debounce fires last.

The URL input is debounced (that's #15456, already on 6.x — not yours), but this clears the value immediately and never cancels what's in flight. Verified against this commit with fake timers:

Populated field. Value https://vimeo.com/1, user edits it to https://vimeo.com/12, then switches to Cloudflare before the debounce fires. Emissions are [[null], ["https://vimeo.com/12"]] — the clear lands, then the stale URL is restored, the field snaps back to URL mode and the provider switch is silently undone.

Empty field. User types a URL then switches provider. this.value is still null here because the debounce hasn't fired, so the if (this.value) guard skips the clear entirely; the debounce then lands the URL and isCloudflare goes false. Same outcome.

Cancelling first fixes both — the pending edit is dropped, and this.value is then accurate:

Suggested change
changeMode(mode) {
if (mode === this.mode) return;
this.mode = mode;
if (this.value) this.update(null);
},
changeMode(mode) {
if (mode === this.mode) return;
this.updateDebounced.cancel();
this.mode = mode;
if (this.value) this.update(null);
},

Worth noting updateCloudflareId below isn't debounced at all, so the ID input reloads the preview iframe on every keystroke while the URL input next to it doesn't.


updateCloudflareId(id) {
this.update(id ? `${CLOUDFLARE_PREFIX}${id}` : null);
},
},

mounted() {
Expand Down
82 changes: 74 additions & 8 deletions resources/js/tests/components/fieldtypes/VideoFieldtype.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@ import { publishContextKey } from '@/components/ui';

window.__ = (key) => key;

let intersect;

beforeEach(() => {
vi.useFakeTimers();

window.IntersectionObserver = class {
constructor(callback) {
intersect = () => callback([{ isIntersecting: true, intersectionRatio: 1 }]);
}
observe() {}
disconnect() {}
};
Expand All @@ -19,23 +24,24 @@ afterEach(() => {
vi.restoreAllMocks();
});

const providers = [
{ value: 'url', label: 'URL' },
{ value: 'cloudflare', label: 'Cloudflare Stream' },
];

const stub = (tag) => ({
props: ['modelValue'],
emits: ['update:modelValue'],
template: `<${tag} :value="modelValue" @input="$emit('update:modelValue', $event.target.value)" />`,
});

const mountVideoField = (props = {}) => {
const mountVideoField = (value = null, video = null) => {
return mount(VideoFieldtype, {
props: {
handle: 'video',
config: {},
meta: {},
...props,
},
props: { handle: 'video', config: {}, meta: { providers, video }, value },
global: {
provide: { [publishContextKey]: {} },
stubs: {
'ui-combobox': stub('select'),
'ui-input': stub('input'),
'ui-input-group': { template: '<div><slot /></div>' },
'ui-input-group-prepend': { template: '<span />' },
Expand All @@ -45,8 +51,68 @@ const mountVideoField = (props = {}) => {
});
};

test('it shows the id input for a cloudflare value', () => {
const wrapper = mountVideoField('cloudflare:abc123');

expect(wrapper.find('input').element.value).toBe('abc123');
});

test('it shows the url input for a url value', () => {
const wrapper = mountVideoField('https://vimeo.com/1');

expect(wrapper.find('input').element.value).toBe('https://vimeo.com/1');
});

test('it stores the cloudflare id with a prefix', async () => {
const wrapper = mountVideoField('cloudflare:old');

await wrapper.find('input').setValue('abc123');

expect(wrapper.emitted('update:value')[0]).toEqual(['cloudflare:abc123']);
});

test('it previews a cloudflare video', async () => {
const wrapper = mountVideoField('cloudflare:abc123');
intersect();
await wrapper.vm.$nextTick();

expect(wrapper.find('iframe').attributes('src')).toBe('https://iframe.cloudflarestream.com/abc123');
});

test('it clears the stored value when switching provider', async () => {
const wrapper = mountVideoField('https://www.youtube.com/watch?v=1234');

await wrapper.find('select').setValue('cloudflare');

expect(wrapper.emitted('update:value')).toHaveLength(1);
expect(wrapper.emitted('update:value')[0]).toEqual([null]);
});

test('it does not emit when switching to the provider already in use', async () => {
const wrapper = mountVideoField(null);

await wrapper.find('select').setValue('url');

expect(wrapper.emitted('update:value')).toBeUndefined();
});

test('it rejects a malformed cloudflare id', async () => {
const wrapper = mountVideoField('cloudflare:abc"><script>alert(1)</script>');
intersect();
await wrapper.vm.$nextTick();

expect(wrapper.find('iframe').exists()).toBe(false);
expect(wrapper.find('p').exists()).toBe(true);
});

test('it uses the provider preloaded in meta for an empty field', () => {
const wrapper = mountVideoField(null, { provider: 'cloudflare', url: null });

expect(wrapper.vm.isCloudflare).toBe(true);
});

test('it debounces updates while typing', async () => {
const wrapper = mountVideoField({ value: null });
const wrapper = mountVideoField(null);
const input = wrapper.find('input');

await input.setValue('https://www.youtube.com/watch?v=1');
Expand Down
18 changes: 18 additions & 0 deletions src/Fieldtypes/Video.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,31 @@
namespace Statamic\Fieldtypes;

use Statamic\Fields\Fieldtype;
use Statamic\Fieldtypes\Video\Embed;

use function Statamic\trans as __;

class Video extends Fieldtype
{
protected $categories = ['media'];

public function augment($value)
{
if (is_null($value)) {
return null;
}

return Embed::fromValue($value);
}

public function preload()
{
return [
'providers' => Embed::options(),
'video' => Embed::fromValue($this->field()->value())->toArray(),
];
}

protected function configFieldItems(): array
{
return [
Expand Down
Loading
Loading