From 73a9790c9e5ca6713265494a305f7ab43135aa27 Mon Sep 17 00:00:00 2001
From: Blesilda Ramirez <ramirezblesilda@gmail.com>
Date: Thu, 20 Jun 2024 20:54:19 +0800
Subject: [PATCH] pkp/pkp-lib#i10033 Implement code review suggestions

---
 .../DropdownActions/DropdownActions.mdx       |  2 +-
 .../DropdownActions.stories.js                | 88 ++++---------------
 .../DropdownActions/DropdownActions.vue       | 64 +++++---------
 3 files changed, 42 insertions(+), 112 deletions(-)

diff --git a/src/components/DropdownActions/DropdownActions.mdx b/src/components/DropdownActions/DropdownActions.mdx
index 5419e0d64..c4db42b59 100644
--- a/src/components/DropdownActions/DropdownActions.mdx
+++ b/src/components/DropdownActions/DropdownActions.mdx
@@ -1,4 +1,4 @@
-import {Primary, Controls, Stories, Meta, Story} from '@storybook/blocks';
+import {Primary, Controls, Stories, Meta, Description} from '@storybook/blocks';
 import DropdownActions from './DropdownActions.vue';
 
 import * as DropdownActionsStories from './DropdownActions.stories.js';
diff --git a/src/components/DropdownActions/DropdownActions.stories.js b/src/components/DropdownActions/DropdownActions.stories.js
index 941c02a5c..6a204fc52 100644
--- a/src/components/DropdownActions/DropdownActions.stories.js
+++ b/src/components/DropdownActions/DropdownActions.stories.js
@@ -11,81 +11,38 @@ export default {
 		},
 		template: '<DropdownActions v-bind="args" />',
 	}),
-	argTypes: {
-		actions: {
-			description:
-				'An array of action objects. Each object should contain `label` (string), `url` (string), an optional `icon` (string) and `isWarnable` (boolean) if the button needs the "warning" button styling from `<Button>` component.',
-			table: {
-				type: {summary: 'Array'},
-				defaultValue: {summary: '[]'},
-			},
-			defaultValue: [],
-		},
-		name: {
-			control: {type: 'text'},
-			table: {
-				type: {summary: 'text'},
-				defaultValue: {summary: ''},
-			},
-			description:
-				"The text label for the button. This is optional. If `isEllipsis` is `false` and `name` is provided, it will be used as the button's label.",
-		},
-		position: {
-			control: {type: 'select'},
-			options: ['left', 'right'],
-			description:
-				'Determines where to show the dropdown button. Options include `left` and `right`.',
-		},
-		isEllipsis: {
-			control: {type: 'boolean'},
-			table: {
-				type: {summary: 'boolean'},
-				defaultValue: {summary: false},
-			},
-			description:
-				'If `true`, the button will display an ellipsis (`...`), and the `ariaLabel` prop must be provided for accessibility.',
-		},
-		ariaLabel: {
-			control: {type: 'text'},
-			table: {
-				type: {summary: 'text'},
-				defaultValue: {summary: ''},
-			},
-			description:
-				'The accessible label for the button, used by screen readers. Required if `isEllipsis` is `true`. If not provided, the component will use the `name` prop or default to "More actions".',
-		},
-	},
+	decorators: [
+		() => ({
+			template:
+				'<div style="height: 300px; padding: 10px;" class="text-center"><story/></div>',
+		}),
+	],
 };
 
 const downloadActions = [
 	{
 		label: 'Author-Only Sections Displayed (PDF)',
-		url: '#',
+		action: 'authorPdf',
 	},
 	{
 		label: 'Author-Only Sections Displayed (XML)',
-		url: '#',
+		action: 'authorXml',
 	},
 	{
 		label: 'Editor Forms Shows All Review Sections (PDF)',
-		url: '#',
+		action: 'editorPdf',
 	},
 	{
 		label: 'Editor Forms Shows All Review Sections (XML)',
-		url: '#',
+		action: 'editorXml',
 	},
 ];
 
 export const Default = {
 	args: {
 		actions: downloadActions,
-		name: 'Download Review Form',
+		label: 'Download Review Form',
 	},
-	decorators: [
-		() => ({
-			template: '<div style="height: 250px"><story/></div>',
-		}),
-	],
 	play: async ({canvasElement}) => {
 		// Assigns canvas to the component root element
 		const canvas = within(canvasElement);
@@ -131,14 +88,9 @@ export const EllipsisMenu = {
 				icon: 'MergeUser',
 			},
 		],
-		isEllipsis: true,
-		ariaLabel: 'User management options',
+		label: 'User management options',
+		displayAsEllipsis: true,
 	},
-	decorators: [
-		() => ({
-			template: '<div style="height: 300px"><story/></div>',
-		}),
-	],
 	play: async ({canvasElement}) => {
 		// Assigns canvas to the component root element
 		const canvas = within(canvasElement);
@@ -148,22 +100,18 @@ export const EllipsisMenu = {
 	},
 };
 
-export const LeftPosition = {
+export const RightAlignedMenu = {
 	args: {
 		actions: downloadActions,
-		name: 'Left Dropdown',
-		position: 'left',
+		label: 'Right Aligned Menu',
+		ariaLabel: 'Click to download content by available formats',
+		direction: 'right',
 	},
-	decorators: [
-		() => ({
-			template: '<div style="height: 250px"><story/></div>',
-		}),
-	],
 	play: async ({canvasElement}) => {
 		// Assigns canvas to the component root element
 		const canvas = within(canvasElement);
 		const user = userEvent.setup();
 
-		await user.click(canvas.getByText('Left Dropdown'));
+		await user.click(canvas.getByText('Right Aligned Menu'));
 	},
 };
diff --git a/src/components/DropdownActions/DropdownActions.vue b/src/components/DropdownActions/DropdownActions.vue
index dacda5185..a8dd9de10 100644
--- a/src/components/DropdownActions/DropdownActions.vue
+++ b/src/components/DropdownActions/DropdownActions.vue
@@ -1,31 +1,22 @@
 <template>
-	<div class="relative flex items-start justify-between">
-		<Menu
-			as="div"
-			:class="
-				position === 'right'
-					? 'ltr:ml-auto rtl:mr-auto'
-					: 'ltr:mr-auto rtl:ml-auto'
-			"
-		>
+	<div class="relative inline-block items-start justify-between">
+		<Menu as="div">
 			<div>
 				<MenuButton
 					class="hover:bg-gray-50 inline-flex w-full justify-center gap-x-1.5 rounded px-3 py-2"
 					:class="
-						shouldUseEllipsis
+						displayAsEllipsis
 							? 'text-3xl-normal'
 							: 'border border-light bg-secondary text-lg-normal'
 					"
+					:aria-label="ariaLabel || null"
 				>
-					<span v-if="!shouldUseEllipsis">{{ name }}</span>
+					<span :class="displayAsEllipsis ? 'sr-only' : ''">{{ label }}</span>
 					<Icon
 						class="-mr-1 h-5 w-5 text-primary"
-						:icon="shouldUseEllipsis ? 'MoreOptions' : 'Dropdown'"
+						:icon="displayAsEllipsis ? 'MoreOptions' : 'Dropdown'"
 						aria-hidden="true"
 					/>
-					<span v-if="shouldUseEllipsis" class="sr-only">
-						{{ computedAriaLabel }}
-					</span>
 				</MenuButton>
 			</div>
 
@@ -38,18 +29,18 @@
 				leave-to-class="transform opacity-0 scale-95"
 			>
 				<MenuItems
-					class="dropdown-shadow absolute z-10 w-max border border-light bg-secondary focus:outline-none"
+					class="absolute z-10 w-max border border-light bg-secondary shadow focus:outline-none"
 					:class="
-						position === 'right'
-							? 'ltr:right-0 ltr:origin-top-right rtl:left-0 rtl:origin-top-right'
-							: 'ltr:left-0 ltr:origin-top-left rtl:right-0 rtl:origin-top-left'
+						direction === 'right'
+							? 'ltr:left-0 ltr:origin-top-left rtl:right-0 rtl:origin-top-left'
+							: 'ltr:right-0 ltr:origin-top-right rtl:left-0 rtl:origin-top-right'
 					"
 				>
 					<MenuItem v-for="(action, i) in actions" :key="i" v-slot="{active}">
 						<div class="min-w-[96px]">
 							<PkpButton
 								v-if="action.label"
-								element="a"
+								:element="action.url ? 'a' : 'button'"
 								:href="action.url"
 								:icon="action.icon"
 								:is-active="active"
@@ -57,6 +48,7 @@
 								:class="i !== actions.length - 1 ? 'border-b' : ''"
 								size-variant="fullWidth"
 								class="border-light"
+								@click="action.name"
 							>
 								{{ action.label }}
 							</PkpButton>
@@ -69,10 +61,10 @@
 </template>
 
 <script setup>
-import {computed} from 'vue';
 import {Menu, MenuButton, MenuItem, MenuItems} from '@headlessui/vue';
 
-const props = defineProps({
+defineProps({
+	/** An array of action objects. Each object should contain `label` (string), `url` (string) or `action` (string), an optional `icon` (string) and `isWarnable` (boolean) if the button needs the "warning" button styling from `<Button>` component. */
 	actions: {
 		type: Array,
 		required: true,
@@ -83,35 +75,25 @@ const props = defineProps({
 			);
 		},
 	},
-	name: {
+	/** The text label for the button. This is required. If `displayAsEllipsis` is `true`, the label will be used for accessibility. */
+	label: {
 		type: String,
-		default: '',
+		required: true,
 	},
-	isEllipsis: {
+	/** If `true`, the button will display an ellipsis (`...`) */
+	displayAsEllipsis: {
 		type: Boolean,
 		default: false,
 	},
+	/** The accessible label for the button, used by screen readers. This is optional. */
 	ariaLabel: {
 		type: String,
 		default: '',
 	},
-	position: {
+	/** This specifies where the dropdown appears relative to the element, such as "left," or "right." */
+	direction: {
 		type: String,
-		default: 'right',
+		default: 'left',
 	},
 });
-
-const computedAriaLabel = computed(() => {
-	return props.ariaLabel || props.name || 'More actions';
-});
-
-const shouldUseEllipsis = computed(() => {
-	return props.isEllipsis || !props.name;
-});
 </script>
-
-<style scoped>
-.dropdown-shadow {
-	box-shadow: 0px 4px 10px 1px rgba(0, 0, 0, 0.5);
-}
-</style>