ronantakizawa/codereview-bench huggingface dataset:
Repository: directus/directus
File: app/src/modules/activity/routes/item.vue (Vue)

Code before:
	} finally {
		loading.value = false;
	}
}

function close() {
	router.push('/activity');
}
</script>

<template>
	<v-drawer :model-value="isOpen" :title="t('activity_item')" @update:model-value="close" @cancel="close">
		<v-progress-circular v-if="loading" indeterminate />

		<div v-else-if="error" class="content">
			<v-notice type="danger">
				{{ error }}
			</v-notice>
		</div>

		<div v-else-if="item" class="content">
			<!-- @TODO add final design -->
			<p class="type-label">{{ t('user') }}:</p>
			<user v-if="item.user" display="name" :value="{ ...item.user, avatar: null }">
				{{ userName(item.user) }}
			</user>

			<p class="type-label">{{ t('action') }}:</p>
			<p>{{ item.action_translated }}</p>

			<p class="type-label">{{ t('date') }}:</p>
			<p>{{ item.timestamp }}</p>

			<p class="type-label">{{ t('ip_address') }}:</p>
			<p>{{ item.ip }}</p>

			<p class="type-label">{{ t('user_agent') }}:</p>
			<p>{{ item.user_agent }}</p>

			<p class="type-label">{{ t('origin') }}:</p>
			<p>{{ item.origin }}</p>

			<p class="type-label">{{ t('collection') }}:</p>
			<p>{{ item.collection }}</p>

			<p class="type-label">{{ t('item') }}:</p>
			<p>{{ item.item }}</p>
		</div>

		<template #actions>
			<v-button v-if="openItemLink" v-tooltip.bottom="t('open')" :to="openItemLink" icon rounded>

Review comment (suggestion):
I don't have push rights to this branch, but wrapping the text in a `<span>` fixes the positioning issue
This gives `v-menu` a DOM element for the activator anchor point

```vue
<user-popover v-if="item.user" :user="item.user.id">
	<span>
		{{ userName(item.user) }}
	</span>
</user-popover>
```

Code after:
		loading.value = false;
	}
}

function close() {
	router.push('/activity');
}
</script>

<template>
	<v-drawer :model-value="isOpen" :title="t('activity_item')" @update:model-value="close" @cancel="close">
		<v-progress-circular v-if="loading" indeterminate />

		<div v-else-if="error" class="content">
			<v-notice type="danger">
				{{ error }}
			</v-notice>
		</div>

		<div v-else-if="item" class="content">
			<!-- @TODO add final design -->
			<p class="type-label">{{ t('user') }}:</p>
			<user-popover v-if="item.user" :user="item.user.id">
				<span>
					{{ userName(item.user) }}
				</span>
			</user-popover>

			<p class="type-label">{{ t('action') }}:</p>
			<p>{{ item.action_translated }}</p>

			<p class="type-label">{{ t('date') }}:</p>
			<p>{{ item.timestamp }}</p>

			<p class="type-label">{{ t('ip_address') }}:</p>
			<p>{{ item.ip }}</p>

			<p class="type-label">{{ t('user_agent') }}:</p>
			<p>{{ item.user_agent }}</p>

			<p class="type-label">{{ t('origin') }}:</p>
			<p>{{ item.origin }}</p>

			<p class="type-label">{{ t('collection') }}:</p>
			<p>{{ item.collection }}</p>

			<p class="type-label">{{ t('item') }}:</p>
			<p>{{ item.item }}</p>
		</div>

		<template #actions>
Repository: directus/directus
File: app/src/components/v-drawer-header.vue (Vue)

Code before:

withDefaults(
	defineProps<{
		title?: string;
		primaryActionIcon?: string;
		small?: boolean;
		shadow?: boolean;
		icon?: string;
		iconColor?: string;
		showBack?: boolean;
	}>(),
	{
		primaryActionIcon: 'menu',
		shadow: false,
	},
);

defineEmits<{
	(e: 'primary'): void;
}>();
</script>

<template>
	<header class="header-bar" :class="{ small, shadow }">

		<PrivateViewHeaderBarIcon v-if="icon || showBack" :icon :show-back :icon-color />

		<div v-if="$slots['title-outer:prepend']" class="title-outer-prepend">
			<slot name="title-outer:prepend" />
		</div>

		<div class="title-container" :class="{ full: !$slots['title-outer:append'] }">
			<div class="headline">
				<slot name="headline" />
			</div>

			<div class="title">
				<slot name="title">
					<slot name="title:prepend" />
					<h1 class="type-title">
						<v-text-overflow :text="title" placement="bottom">{{ title }}</v-text-overflow>
					</h1>
					<slot name="title:append" />
				</slot>
			</div>

			<slot name="title-outer:append" />
		</div>

		<div class="spacer" />


Review comment (suggestion):
Given that we're re-using this here, I'm wondering if there's a better scoping than the broad component lib

Code after:

withDefaults(
	defineProps<{
		title?: string;
		shadow?: boolean;
		icon?: string;
		iconColor?: string;
		showBack?: boolean;
	}>(),
	{
		shadow: false,
	},
);
</script>

<template>
	<header class="header-bar" :class="{ shadow }">
		<private-view-header-bar-icon v-if="icon || showBack" :icon :show-back :icon-color />

		<div v-if="$slots['title-outer:prepend']" class="title-outer-prepend">
			<slot name="title-outer:prepend" />
		</div>

		<div class="title-container" :class="{ full: !$slots['title-outer:append'] }">
			<div class="headline">
				<slot name="headline" />
			</div>

			<div class="title">
				<slot name="title">
					<slot name="title:prepend" />
					<h1 class="type-title">
						<v-text-overflow :text="title" placement="bottom">{{ title }}</v-text-overflow>
					</h1>
					<slot name="title:append" />
				</slot>
			</div>

			<slot name="title-outer:append" />
		</div>

		<div class="spacer" />

		<slot name="actions:prepend" />

		<header-bar-actions>
			<slot name="actions" />
		</header-bar-actions>

		<slot name="actions:append" />
	</header>
Repository: directus/directus
File: app/src/components/v-drawer.vue (Vue)

Code before:
				icon
				rounded
				secondary
				@click="$emit('cancel')"
			>
				<v-icon name="close" />
			</v-button>

			<div class="content">
				<v-overlay v-if="$slots.sidebar" absolute />

				<v-resizeable
					v-if="$slots.sidebar"
					:disabled="!sidebarResizeable"
					:width="sidebarWidth"
					:max-width="sidebarMaxWidth"
				>
					<nav class="sidebar">
						<div class="sidebar-content">
							<slot name="sidebar" />
						</div>
					</nav>
				</v-resizeable>

				<main ref="scrollContainer" :class="{ main: true, 'small-search-input': $slots.sidebar }">
					<v-drawer-header
						:title="title"
						primary-action-icon="close"
						:shadow="showHeaderShadow"
						:icon="icon"
						:icon-color="iconColor"
						@primary="$emit('cancel')"
					>
						<template #title><slot name="title" /></template>
						<template #headline>
							<slot name="subtitle">
								<p v-if="subtitle" class="subtitle">{{ subtitle }}</p>
							</slot>
						</template>

						<template #title-outer:prepend>
							<slot name="title-outer:prepend" />
						</template>

						<template #actions:prepend><slot name="actions:prepend" /></template>
						<template #actions><slot name="actions" /></template>

						<template #title:append><slot name="header:append" /></template>
					</v-drawer-header>

					<v-detail v-if="$slots.sidebar" class="mobile-sidebar" :label="sidebarLabel">

Review comment (suggestion):
Lets rework these to an imported `VDrawerHeader` for better typing

Code after:
				icon
				rounded
				secondary
				@click="$emit('cancel')"
			>
				<v-icon name="close" />
			</v-button>

			<div class="content">
				<v-overlay v-if="$slots.sidebar" absolute />

				<v-resizeable
					v-if="$slots.sidebar"
					:disabled="!sidebarResizeable"
					:width="sidebarWidth"
					:max-width="sidebarMaxWidth"
				>
					<nav class="sidebar">
						<div class="sidebar-content">
							<slot name="sidebar" />
						</div>
					</nav>
				</v-resizeable>

				<main ref="scrollContainer" :class="{ main: true, 'small-search-input': $slots.sidebar }">
					<v-drawer-header
						:title="title"
						:shadow="showHeaderShadow"
						:icon="icon"
						:icon-color="iconColor"
					>
						<template #title><slot name="title" /></template>
						<template #headline>
							<slot name="subtitle">
								<p v-if="subtitle" class="subtitle">{{ subtitle }}</p>
							</slot>
						</template>

						<template #title-outer:prepend>
							<slot name="title-outer:prepend" />
						</template>

						<template #actions:prepend><slot name="actions:prepend" /></template>
						<template #actions><slot name="actions" /></template>

						<template #title:append><slot name="header:append" /></template>
					</v-drawer-header>

					<v-detail v-if="$slots.sidebar" class="mobile-sidebar" :label="sidebarLabel">
						<nav>
							<slot name="sidebar" />
Repository: directus/directus
File: app/src/components/v-input.vue (Vue)

Code before:
	let value = target.value;

	setInvalidInput(target);

	if (props.nullable === true && value === '') {
		emit('update:modelValue', null);
		return;
	}

	if (props.type === 'number') {
		const parsedNumber = Number(value);

		if (props.integer === true && !Number.isInteger(parsedNumber) && Number.isNaN(parsedNumber) === false) {
			emit('update:modelValue', Math.floor(parsedNumber));
			return;
		}

		// Ignore if numeric value remains unchanged
		if (props.modelValue !== parsedNumber) {
			emit('update:modelValue', parsedNumber);
		}
	} else {
		// decimal input
		if (props.float === true) {
			/**
			 * Normalize decimal separator to '.'
			 * Thousands separators are not supported in the input
			 */
			value = value.replace(',', '.');
		}

		if (props.slug === true) {
			const endsWithSpace = value.endsWith(' ');
			value = slugify(value, { separator: props.slugSeparator, preserveTrailingDash: true });
			if (endsWithSpace) value += props.slugSeparator;
		}

		if (props.dbSafe === true) {
			value = value.replace(/\s/g, '_');
			// Replace é -> e etc
			value = value.normalize('NFD').replace(/[\u0300-\u036f]/g, '');
			// prevent pasting of non dbSafeCharacters from bypassing the keydown checks
			value = value.replace(/[^a-zA-Z0-9_]/g, '');
		}

		emit('update:modelValue', value);
	}
}

function stepUp() {
	if (!input.value) return;

Review comment (suggestion):
```suggestion
			 * Normalize decimal separator from ',' to '.'
```

Code after:
	let value = target.value;

	setInvalidInput(target);

	if (props.nullable === true && value === '') {
		emit('update:modelValue', null);
		return;
	}

	if (props.type === 'number') {
		const parsedNumber = Number(value);

		if (props.integer === true && !Number.isInteger(parsedNumber) && Number.isNaN(parsedNumber) === false) {
			emit('update:modelValue', Math.floor(parsedNumber));
			return;
		}

		// Ignore if numeric value remains unchanged
		if (props.modelValue !== parsedNumber) {
			emit('update:modelValue', parsedNumber);
		}
	} else {
		// decimal input marked as text
		if (props.float === true) {
			/**
			 * Normalize decimal separator from ',' to '.'
			 * Thousands separators are not supported in the input
			 */
			value = value.replace(',', '.');
		}

		if (props.slug === true) {
			const endsWithSpace = value.endsWith(' ');
			value = slugify(value, { separator: props.slugSeparator, preserveTrailingDash: true });
			if (endsWithSpace) value += props.slugSeparator;
		}

		if (props.dbSafe === true) {
			value = value.replace(/\s/g, '_');
			// Replace é -> e etc
			value = value.normalize('NFD').replace(/[\u0300-\u036f]/g, '');
			// prevent pasting of non dbSafeCharacters from bypassing the keydown checks
			value = value.replace(/[^a-zA-Z0-9_]/g, '');
		}

		emit('update:modelValue', value);
	}
}

function stepUp() {
	if (!input.value) return;
Repository: directus/directus
File: app/src/components/v-input.vue (Vue)

Code before:

function emitValue(event: InputEvent) {
	const target = event.target as HTMLInputElement;
	let value = target.value;

	setInvalidInput(target);

	if (props.nullable === true && value === '') {
		emit('update:modelValue', null);
		return;
	}

	if (props.type === 'number') {
		const parsedNumber = Number(value);

		if (props.integer === true && !Number.isInteger(parsedNumber) && Number.isNaN(parsedNumber) === false) {
			emit('update:modelValue', Math.floor(parsedNumber));
			return;
		}

		// Ignore if numeric value remains unchanged
		if (props.modelValue !== parsedNumber) {
			emit('update:modelValue', parsedNumber);
		}
	} else {
		// decimal input
		if (props.float === true) {
			/**
			 * Normalize decimal separator from ',' to '.'
			 * Thousands separators are not supported in the input
			 */
			value = value.replace(',', '.');
		}

		if (props.slug === true) {
			const endsWithSpace = value.endsWith(' ');
			value = slugify(value, { separator: props.slugSeparator, preserveTrailingDash: true });
			if (endsWithSpace) value += props.slugSeparator;
		}

		if (props.dbSafe === true) {
			value = value.replace(/\s/g, '_');
			// Replace é -> e etc
			value = value.normalize('NFD').replace(/[\u0300-\u036f]/g, '');
			// prevent pasting of non dbSafeCharacters from bypassing the keydown checks
			value = value.replace(/[^a-zA-Z0-9_]/g, '');
		}

		emit('update:modelValue', value);
	}
}

Review comment (suggestion):
```suggestion
		// decimal input marked as text
```

Code after:

function emitValue(event: InputEvent) {
	const target = event.target as HTMLInputElement;
	let value = target.value;

	setInvalidInput(target);

	if (props.nullable === true && value === '') {
		emit('update:modelValue', null);
		return;
	}

	if (props.type === 'number') {
		const parsedNumber = Number(value);

		if (props.integer === true && !Number.isInteger(parsedNumber) && Number.isNaN(parsedNumber) === false) {
			emit('update:modelValue', Math.floor(parsedNumber));
			return;
		}

		// Ignore if numeric value remains unchanged
		if (props.modelValue !== parsedNumber) {
			emit('update:modelValue', parsedNumber);
		}
	} else {
		// decimal input marked as text
		if (props.float === true) {
			/**
			 * Normalize decimal separator from ',' to '.'
			 * Thousands separators are not supported in the input
			 */
			value = value.replace(',', '.');
		}

		if (props.slug === true) {
			const endsWithSpace = value.endsWith(' ');
			value = slugify(value, { separator: props.slugSeparator, preserveTrailingDash: true });
			if (endsWithSpace) value += props.slugSeparator;
		}

		if (props.dbSafe === true) {
			value = value.replace(/\s/g, '_');
			// Replace é -> e etc
			value = value.normalize('NFD').replace(/[\u0300-\u036f]/g, '');
			// prevent pasting of non dbSafeCharacters from bypassing the keydown checks
			value = value.replace(/[^a-zA-Z0-9_]/g, '');
		}

		emit('update:modelValue', value);
	}
}
Repository: directus/directus
File: app/src/layouts/cards/cards.vue (Vue)

Code before:

const { width: innerWidth } = useElementSize(layoutElement);

const { sizes: pageSizes, selected: selectedSize } = usePageSize<string>(
	[25, 50, 100, 250, 500, 1000],
	(value) => String(value),
	props.limit,
);

if (limitWritable.value !== selectedSize) {
	limitWritable.value = selectedSize;
}

watch(
	() => props.page,
	() => mainElement!.value?.scrollTo({ top: 0, behavior: 'smooth' }),
);

watch(innerWidth, (value) => {
	emit('update:width', value);
});
</script>

<template>
	<div ref="layoutElement" class="layout-cards" :style="{ '--size': size * 40 + 'px' }">
		<template v-if="loading || ((items.length ?? 0) > 0 && !error)">
			<cards-header
				v-model:size="sizeWritable"
				v-model:selection="selectionWritable"
				v-model:sort="sortWritable"
				:fields="fieldsInCollection"
				:show-select="showSelect"
				@select-all="selectAll"
			/>

			<v-progress-circular v-if="loading" indeterminate rounded />

			<div v-else class="grid" :class="{ 'single-row': isSingleRow }">
				<card
					v-for="item in items"
					:key="item[primaryKeyField!.field]"
					v-model="selectionWritable"
					:item-key="primaryKeyField!.field"
					:crop="imageFit === 'crop'"
					:icon="icon"
					:file="imageSource ? item[imageSource] : null"
					:item="item"
					:select-mode="selectMode || (selection && selection.length > 0)"
					:to="getLinkForItem(item)"
					:readonly="readonly"
				>

Review comment (suggestion):
```suggestion
		<template v-if="loading || (items.length > 0 && !error)">
```

This assertion reads a bit strangely. Any reason you didn't just go with `items.length > 0`. It's a required prop, so it should always at least be an empty array I think.

Code after:

const { width: innerWidth } = useElementSize(layoutElement);

const { sizes: pageSizes, selected: selectedSize } = usePageSize<string>(
	[25, 50, 100, 250, 500, 1000],
	(value) => String(value),
	props.limit,
);

if (limitWritable.value !== selectedSize) {
	limitWritable.value = selectedSize;
}

watch(
	() => props.page,
	() => mainElement!.value?.scrollTo({ top: 0, behavior: 'smooth' }),
);

watch(innerWidth, (value) => {
	emit('update:width', value);
});
</script>

<template>
	<div ref="layoutElement" class="layout-cards" :style="{ '--size': size * 40 + 'px' }">
		<template v-if="loading || (items.length > 0 && !error)">
			<cards-header
				v-model:size="sizeWritable"
				v-model:selection="selectionWritable"
				v-model:sort="sortWritable"
				:fields="fieldsInCollection"
				:show-select="showSelect"
				@select-all="selectAll"
			/>

			<v-progress-circular v-if="loading" indeterminate rounded />

			<div v-else class="grid" :class="{ 'single-row': isSingleRow }">
				<card
					v-for="item in items"
					:key="item[primaryKeyField!.field]"
					v-model="selectionWritable"
					:item-key="primaryKeyField!.field"
					:crop="imageFit === 'crop'"
					:icon="icon"
					:file="imageSource ? item[imageSource] : null"
					:item="item"
					:select-mode="selectMode || (selection && selection.length > 0)"
					:to="getLinkForItem(item)"
					:readonly="readonly"
				>
Repository: directus/directus
File: app/src/layouts/tabular/tabular.vue (Vue)

Code before:
	[25, 50, 100, 250, 500, 1000],
	(value) => String(value),
	props.limit,
);

if (limitWritable.value !== selectedSize) {
	limitWritable.value = selectedSize;
}

const fieldsWritable = useSync(props, 'fields', emit);

const { getFromAliasedItem } = useAliasFields(fieldsWritable, collection);

function addField(fieldKey: string) {
	fieldsWritable.value = [...fieldsWritable.value, fieldKey];
}

function removeField(fieldKey: string) {
	fieldsWritable.value = fieldsWritable.value.filter((field) => field !== fieldKey);
}
</script>

<template>
	<div class="layout-tabular">
		<v-table
			v-if="loading || ((items.length ?? 0) > 0 && !error)"
			ref="table"
			v-model="selectionWritable"
			v-model:headers="tableHeadersWritable"
			class="table"
			fixed-header
			:show-select="showSelect ? showSelect : selection !== undefined"
			show-resize
			must-sort
			:sort="tableSort"
			:items="items"
			:loading="loading"
			:row-height="tableRowHeight"
			:item-key="primaryKeyField?.field"
			:show-manual-sort="sortAllowed"
			:manual-sort-key="sortField"
			allow-header-reorder
			selection-use-keys
			@click:row="onRowClick"
			@update:sort="onSortChange"
			@manual-sort="changeManualSort"
		>
			<template v-for="header in tableHeaders" :key="header.value" #[`item.${header.value}`]="{ item }">
				<render-display
					:value="getFromAliasedItem(item, header.value)"
					:display="header.field.display"

Review comment (suggestion):
Same note about assertion

```suggestion
			v-if="loading || (items.length > 0 && !error)"
```

Code after:
	[25, 50, 100, 250, 500, 1000],
	(value) => String(value),
	props.limit,
);

if (limitWritable.value !== selectedSize) {
	limitWritable.value = selectedSize;
}

const fieldsWritable = useSync(props, 'fields', emit);

const { getFromAliasedItem } = useAliasFields(fieldsWritable, collection);

function addField(fieldKey: string) {
	fieldsWritable.value = [...fieldsWritable.value, fieldKey];
}

function removeField(fieldKey: string) {
	fieldsWritable.value = fieldsWritable.value.filter((field) => field !== fieldKey);
}
</script>

<template>
	<div class="layout-tabular">
		<v-table
			v-if="loading || (items.length > 0 && !error)"
			ref="table"
			v-model="selectionWritable"
			v-model:headers="tableHeadersWritable"
			class="table"
			fixed-header
			:show-select="showSelect ? showSelect : selection !== undefined"
			show-resize
			must-sort
			:sort="tableSort"
			:items="items"
			:loading="loading"
			:row-height="tableRowHeight"
			:item-key="primaryKeyField?.field"
			:show-manual-sort="sortAllowed"
			:manual-sort-key="sortField"
			allow-header-reorder
			selection-use-keys
			@click:row="onRowClick"
			@update:sort="onSortChange"
			@manual-sort="changeManualSort"
		>
			<template v-for="header in tableHeaders" :key="header.value" #[`item.${header.value}`]="{ item }">
				<render-display
					:value="getFromAliasedItem(item, header.value)"
					:display="header.field.display"
Repository: directus/directus
File: api/src/utils/get-address.test.ts (TypeScript)

Code before:
describe('getAddress', async () => {
	const serverHost = '127.0.0.1';
	const serverSocket = '/tmp/server-test.sock';
	const serverPort = await getPort();

	test('Should return unix socket before server is listening when path is provided', async () => {
		const server = await createServer();

		vi.mocked(useEnv).mockReturnValue({
			UNIX_SOCKET_PATH: serverSocket,
		});

		expect(getAddress(server)).toBe(serverSocket);
		server.close();
	});

	test('Should return host + port before server is listening when path is undefined', async () => {
		const server = await createServer();

		vi.mocked(useEnv).mockReturnValue({
			PORT: serverPort,
			HOST: serverHost,
		});

		expect(getAddress(server)).toBe(`${serverHost}:${serverPort}`);
		server.close();
	});

	test('Should return unix socket when path is provided', async () => {
		const server = await createServer({ path: serverSocket });

		expect(getAddress(server)).toBe(serverSocket);
		server.close();
	});

	test('Should return host + port when path is undefined', async () => {
		const server = await createServer({ host: serverHost, port: serverPort });

		expect(getAddress(server)).toBe(`${serverHost}:${serverPort}`);
		server.close();
	});
});


Review comment (refactor):
Since all tests use server.close(), we could refactor this with an afterEach:

```ts
afterEach(() => {
  server?.close();
});
```

Code after:
describe('getAddress', async () => {
	const serverHost = '127.0.0.1';
	const serverSocket = '/tmp/server-test.sock';
	const serverPort = await getPort();

	let server: http.Server | undefined;

	afterEach(async () => {
		server?.close();
		server = undefined;
	});

	test('Should return unix socket before server is listening when path is provided', async () => {
		server = await createServer();

		vi.mocked(useEnv).mockReturnValue({
			UNIX_SOCKET_PATH: serverSocket,
		});

		expect(getAddress(server)).toBe(serverSocket);
	});

	test('Should return host + port before server is listening when path is undefined', async () => {
		server = await createServer();

		vi.mocked(useEnv).mockReturnValue({
			PORT: serverPort,
			HOST: serverHost,
		});

		expect(getAddress(server)).toBe(`${serverHost}:${serverPort}`);
	});

	test('Should return unix socket when path is provided', async () => {
		server = await createServer({ path: serverSocket });

		expect(getAddress(server)).toBe(serverSocket);
	});

	test('Should return host + port when path is undefined', async () => {
		server = await createServer({ host: serverHost, port: serverPort });

		expect(getAddress(server)).toBe(`${serverHost}:${serverPort}`);
	});
});