pkp/pkp-lib#11296, pkp/pkp-lib#11366, pkp/pkp-lib#11387 [Versioning] #621

blesildaramirez · 2025-05-15T05:20:09Z

No description provided.

jardakotesovec

Hi Blessie,
looking great, first part of the feedback is done.

In addition to that I am now looking what we could do that form. Will post that separately, need to do some experimenting first.

jardakotesovec · 2025-05-19T08:11:19Z

src/components/Modal/DialogBody.vue

@@ -62,3 +65,14 @@ function fireCallback(callback) {
 	}
 }
 </script>
+
+<style>
+/* Override style when form is opened in a dialog */


I know that we still have similar patterns in the code base, but trying to move away from them and basically don't change styling of the component from the outside, and handle these scenarios via variants. Reason basically is that it makes difficult to change the underlying components - if there are these hidden dependencies from parent components.

Here I think biggest problem is additional padding on the sides. So maybe just full width variant would work ok?

src/composables/useForm.js

jardakotesovec · 2025-05-19T08:37:04Z

src/composables/useModal.js

+	 * Close the currently open dialog modal
+	 * @param {boolean} [triggerLegacyCloseHandler=true] - Whether to trigger the legacy close handler
+	 */
+	function closeDialog(triggerLegacyCloseHandler = true) {


Lets keep useModal friendly and remove that mysterious argument, I think its unnecessary. Its only important within modalStore to differentiate whether the close event is coming from the legacy handler (therefore no need to call him again) or not. And here we already know that its not coming from legacy handler

jardakotesovec · 2025-05-19T08:54:15Z

src/managers/FileManager/useFileManagerConfig.js

@@ -10,6 +10,7 @@ export const FileManagerConfigurations = {
 			{
 				roles: [pkp.const.ROLE_ID_AUTHOR],
 				actions: [
+					Actions.FILE_SEND_TO_EDITOR,


Do we have details whether for author this functionality needs to be bit different. Do we let Author create new version etc?

Hi Jarda, this was confirmed that we should only allow "Send File to Text Editor" for users who can also access "Publish, Unpublish" or "Create New Version". So I updated this and applied accordingly.

jardakotesovec · 2025-05-19T09:47:48Z

src/pages/workflow/composables/useWorkflowMenu.js

 	 */
 	const selectedMenuState = computed(() => {
 		const actionArgs = selectedMenuItem.value?.actionArgs || {};

+		// If menu is "dialog-only" action, don't update the actionArgs
+		// to preserve the current primary/secondary menu items dislayed in the workflow content
+		if (actionArgs.isDialogOnly) {


I was thinking whether this is good excuse to actually fix that stupid selectMenu action, which does not exist. And thats is super confusing.. and would like to fix that.

What do you think about these changes #622

With that, basically if you add action without state, it would just get triggered, but it would not select that menu item. So you would not need to track the previousMenuState here.

jardakotesovec · 2025-05-19T09:50:51Z

src/pages/workflow/composables/useWorkflowMenu.js

@@ -68,7 +93,15 @@ export function useWorkflowMenu({

 	// Update selectedMenuKey in url when menu selection changes
 	watch(selectedMenuKey, (newSelectedMenuKey) => {
-		queryParamsUrl.workflowMenuKey = newSelectedMenuKey;
+		if (newSelectedMenuKey !== 'publication_create_new_version') {
+			queryParamsUrl.workflowMenuKey = newSelectedMenuKey;


This I believe also could go away with the PR I suggested above.

jardakotesovec · 2025-05-19T10:13:02Z

src/pages/workflow/composables/useWorkflowVersionForm.js

+		};
+	};
+
+	const handleVersionSubmission = async (formData) => {


Nothing wrong with using arrow function, just to be bit more consistent with rest of the composables, where we use normal function

async function handleVersionSubmission(formData){...

blesildaramirez · 2025-05-26T19:17:14Z

Hi @jardakotesovec , thank you for all your inputs! This is now ready for your review again.

jardakotesovec

Hi Blessie, thank you for updates. Couple minor items.

jardakotesovec · 2025-05-28T08:35:00Z

src/components/Form/Form.vue

@@ -143,6 +145,8 @@ export default {
 				return true;
 			},
 		},
+		spacingVariant: String,


Can you add comment here to clarify the options, and when someone would use them?

Actually not sure about the footerSpacingVariant purpose?

There's some styling that FormPage.vue adds to the bottom of the form, the footerSpacingVariant will change that to match the styling with Dialog. But I removed this now and just kept the spacingVariant prop to handle two computed classes for styling.

jardakotesovec · 2025-05-28T09:04:02Z

src/managers/FileManager/useFileManagerActions.js

+			bodyProps: {
+				mode: 'sendToTextEditor',
+				displayDefaultFooter: false,
+				onCloseFn: () => closeDialog(false),


I think the boolean argument is not needed anymore after removing it from closeDialog in useModal?

jardakotesovec · 2025-05-28T09:05:45Z

src/pages/workflow/composables/useWorkflowVersionForm.js

+	const {getLatestPublication} = useSubmission();
+	let publications = [];
+	let latestPublication = null;
+	const modeState = {


Here I would suggest some comments, so it easier to understand what the modes represents. That should make reading rest of the logic easier.

jardakotesovec · 2025-05-28T09:17:13Z

src/composables/useForm.js

@@ -424,5 +447,7 @@ export function useForm(_form = {}, {customSubmit} = {}) {
 		addGroup,
 		addFieldText,
 		addFieldSelect,
+		getField,


All these functions make sense to not accept form argument. You can probably just move the getField inside the useForm so it does not need form as argument.

jardakotesovec · 2025-05-28T09:56:15Z

src/pages/workflow/composables/useWorkflowVersionForm.js

+			option.value === 'true' ? {...option, disabled: !allowMinor} : option,
+		);
+
+		setSelectOptions(versionIsMinorField, updatedOptions);


Sorry to obsess about this. But keep thinking about how to structure the useForm api to scale across different use cases. Here you are manipulating existing options array, already tackling internal field structure. So the setSelectOptions is not really abstracting much.

I am thinking about options we could go about this to set good example for future use cases..

Basically to solve the case the need of updating anything on the form field based on some scenario.

To avoid duplications with creating udpate* functions, like updateTextField in addition to addTextField. Maybe we could just tweak the add* functions so they replace field, if it already exist (that can be checked by the field name).

With that, you could follow the declarative approach as vue.js has, having function which gives you result based on the input state.

function getVersionIsMinorField({allowMinor, modelState, currentValue}) { return { label: t('publication.revisionSignificance.label'), description: t('publication.revisionSignificance.description'), options: [ {label: t('publication.revisionSignificance.major'), value: 'false'}, { label: t('publication.revisionSignificance.minor'), value: 'true', disabled: !allowMinor, }, ], size: 'large', isRequired: modeState.isPublishMode, value: currentValue === 'true' && !allowMinor ? 'false' : currentValue, }; }

Which you could use for both initially adding the field and for updating it in updateMinorOptionAvailability

addFieldSelect('versionIsMinor', getVersionIsMinorField({...}))

What you think?

Just to expand on that suggestion. We will be adding third parameter options to the addField* functions. So we can specify where it should be added for example (we have similar capability in PHP, and it will be useful to plugins to be able to insert field to certain place). So maybe we can also have override:true option there so be more explicit about intention to update existing field.

Hi Jarda, this makes sense. I updated this and if you're happy with the changes on the addFieldSelect, I can apply the same on the other addField functions.

jardakotesovec · 2025-05-28T09:59:25Z

src/pages/workflow/composables/useWorkflowMenu.js

@@ -66,11 +81,6 @@ export function useWorkflowMenu({
 		}
 	});

-	// Update selectedMenuKey in url when menu selection changes
-	watch(selectedMenuKey, (newSelectedMenuKey) => {


Why this got removed? This it stops updating the currently selected menuKey in the url, which defeats the idea of being able to bookmark or share the url and open the workflow menu in given place

The entire watch function was accidentally removed after I reverted my changes intended for the checking if newSelectedMenuKey === 'publication_create_new_version', which is now deleted. I brought this back now.

…ssion workflow

…nges from side menu in submission workflow

… data is fetched

…in publication pages

…tPublication id when loading submission page

… defined

…s to useWorkflowDataSubmissionPublication api

…menu clicked is dialog only

…lication version

…orkflowVersionDialogBody.vue and define field values depending on form mode

…ated

…iles

…es not exist on any publications yet

… component

…ctions api

…enu for publications

…enu for publcation versions

…ore publishing unassigned versions

…publish access

…unctions

…modal

…o handle styling for forms

…an access publish, unpublish or create new version

…footer container

…lication's Title & Abstract page

…orm argument

…s and remove setSelectOptions method

…MenuKey

…er for OMP and OPS

…ation date

jardakotesovec

Looking great.. only thing I think still would be worthwhile improving was more detailed explanation where the individual mode are used. So its easier to understand in what contexts its used. But thats just minor documenting thing.

I will improve useForm at some point for more fields and to handle override in more general way. But for this PR this is ok.

I think great call on concentrating the logic for this to separate composable - I think that good use of composables.

jardakotesovec requested changes May 19, 2025

View reviewed changes

blesildaramirez force-pushed the i11296-v2 branch from 0e41b34 to cebf298 Compare May 26, 2025 04:58

jardakotesovec requested changes May 28, 2025

View reviewed changes

blesildaramirez force-pushed the i11296-v2 branch 2 times, most recently from 3b7562e to dd82bc1 Compare May 28, 2025 19:12

blesildaramirez added 21 commits May 29, 2025 19:43

pkp/pkp-lib#11296 Add publication versions on the left menu for submi…

1455c49

…ssion workflow

pkp/pkp-lib#11296 Change selected publication when publication id cha…

a28b963

…nges from side menu in submission workflow

pkp/pkp-lib#11296 Add expandedKeys in useSideMenu returned data

d1f01a5

pkp/pkp-lib#11296 Add publication id in expanded keys when submission…

0e6c196

… data is fetched

pkp/pkp-lib#11296 Remove Versions dropdown Create New Version button …

1800138

…in publication pages

pkp/pkp-lib#11296 Expand latest publication version instead of curren…

c3fb718

…tPublication id when loading submission page

pkp/pkp-lib#11296 Add closeDialog method in useModal composable api

91145eb

pkp/pkp-lib#11296 Update dialog body footer styling if no actions are…

47dcfc7

… defined

pkp/pkp-lib#11296 Move createNewVersion method from useWorkflowAction…

fe2bb7b

…s to useWorkflowDataSubmissionPublication api

pkp/pkp-lib#11296 Add create new version dialog form

b286b15

pkp/pkp-lib#11296 Preserve workflow primary and secondary items when …

9799d95

…menu clicked is dialog only

pkp/pkp-lib#11296 Add options to Versions field when creating new pub…

20c2ef5

…lication version

pkp/pkp-lib#11296 Add setSelectOptions method to useForm api

481d4f0

pkp/pkp-lib#11296 Rename WorkflowCreateNewVersionDialogBody.vue to /W…

871d30c

…orkflowVersionDialogBody.vue and define field values depending on form mode

pkp/pkp-lib#11296 Redirect to new publication once new version is cre…

69abb34

…ated

pkp/pkp-lib#11366 Add UpdateFile icon

f14c6af

pkp/pkp-lib#11366 Add locale keys for send file to text editor UI

43c904a

pkp/pkp-lib#11366 Add Send File to Text Editor action to submission f…

c585ee8

…iles

pkp/pkp-lib#11296 Move form handler to useWorkflowVersionForm api

559e8db

pkp/pkp-lib#11296 Disable minor revision if version stage selected do…

6ce0830

…es not exist on any publications yet

pkp/pkp-lib#11296 Improve form fields handling for version form

24e7811

blesildaramirez added 21 commits May 29, 2025 19:43

pkp/pkp-lib#11366 Remove onSubmitFn prop in WorkflowVersionDialogBody…

37cb9bb

… component

pkp/pkp-lib#11366 Move form submit handler to useWorkflowVersionForm api

9c57166

pkp/pkp-lib#11296 Move createNewVersion function back to useWorkflowA…

0a709fa

…ctions api

pkp/pkp-lib#11296 Change OMP navigation config to apply collapsible m…

f5f1e86

…enu for publications

pkp/pkp-lib#11296 Change OPS navigation config to apply collapsible m…

ac8ac12

…enu for publcation versions

pkp/pkp-lib#11296 Add field update methods to useForm api

ba27dae

pkp/pkp-lib#11387 Add side modal form to select publication stage bef…

75232c4

…ore publishing unassigned versions

pkp/pkp-lib#11296 Remove unused methods in useForm api

372ad80

pkp/pkp-lib#11296 Define versionStageOptions in workflow store

bef1f54

pkp/pkp-lib#11296 Fix renamed menu state in useWorkflowMenu api

ede3940

pkp/pkp-lib#11296 Use useForm for defining Version form config

43b5414

pkp/pkp-lib#11296 Only show Create New Version button for users with …

2303b8e

…publish access

pkp/pkp-lib#11296 Simplify workflow menu state handling and improve f…

c29d118

…unctions

pkp/pkp-lib#11387 Fix set function for handling version form in side …

4a9507a

…modal

pkp/pkp-lib#11296 Add spacingVariant and footerSpacingVariant props t…

24403b8

…o handle styling for forms

pkp/pkp-lib#11296 Define default value to versionStageOptions

aa40ba4

pkp/pkp-lib#11296 Update mock data for publications

e420dcc

pkp/pkp-lib#11366 Offer Send File to Text Editor only for users who c…

ecc1c77

…an access publish, unpublish or create new version

pkp/pkp-lib#11296 Add prop displayDefaultFooter for removing default …

d2b892f

…footer container

pkp/pkp-lib#11296 Fix Schedule for Publication not opening latest pub…

c554692

…lication's Title & Abstract page

pkp/pkp-lib#11296 Remove use of publication.version locale key

988347b

blesildaramirez force-pushed the i11296-v2 branch from dd82bc1 to 988347b Compare May 29, 2025 23:44

blesildaramirez added 7 commits May 30, 2025 00:02

pkp/pkp-lib#11296 Remove footerSpacingVariant prop

89e562b

pkp/pkp-lib#11296 Remove closeDialog param

1f8e1c1

pkp/pkp-lib#11296 Move getField method inside useForm api to remove f…

772a24f

…orm argument

pkp/pkp-lib#11296 Update addFieldSelect to handle select field update…

9a859de

…s and remove setSelectOptions method

pkp/pkp-lib#11296 Revert deleted watch function for handling workflow…

3448453

…MenuKey

pkp/pkp-lib#11296 Remove Create New Version button from workflow head…

a39792a

…er for OMP and OPS

pkp/pkp-lib#11296 Sort publications list by versionStage data and cre…

7574be4

…ation date

jardakotesovec approved these changes Jun 9, 2025

View reviewed changes

pkp/pkp-lib#11296, pkp/pkp-lib#11366, pkp/pkp-lib#11387 [Versioning] #621

Are you sure you want to change the base?

pkp/pkp-lib#11296, pkp/pkp-lib#11366, pkp/pkp-lib#11387 [Versioning] #621

Conversation

blesildaramirez commented May 15, 2025

Uh oh!

jardakotesovec left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blesildaramirez commented May 26, 2025

Uh oh!

jardakotesovec left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jardakotesovec May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jardakotesovec left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jardakotesovec May 28, 2025 •

edited

Loading

jardakotesovec left a comment •

edited

Loading