pkp/pkp-lib#11327 Implement public facing UI for user comments #659

taslangraham · 2025-07-09T05:27:02Z

src/components/UserComment/UserComment.vue

ewhanson

Thanks @taslangraham. I'll let @jardakotesovec comment on anything specific about the Vue implementations, but it overall looks good to me. Just a few questions.

ewhanson · 2025-08-18T22:13:15Z

src/frontend/components/PkpUserComment/PkpUserCommentReportModal.vue

@@ -0,0 +1,108 @@
+<template>
+	<PkpModalBody>


The close button isn't highlighting and being treated as a clickable element by the mouse. It's still keyboard navigable, but it's not receiving the typical visual feedback from the pointer.

ewhanson · 2025-08-18T22:15:39Z

src/frontend/components/PkpUserComment/PkpUserCommentsList.vue

+
+		<div v-if="isLatestPublication && !!currentUser">
+			<PkpTextarea
+				placeholder="What do you think about this publication? Type your comments here."


The placeholder should probably be localized too.

Yes.. and also label is essential for accessibility.
So we need to pass the same text as label. Because screen readers are somewhat inconsistent how they present placeholder.. And add prop on PkpTextarea to hide it (only for screenreaders).

jardakotesovec

To give you heads up. After we merge this version.. I will start gradually refining some of the components there.. and possibly reorganise things once I have good styling strategy settled. I would probably make these adjustments myself to see it in practice and just ask you for review. Lets see how that goes.

src/frontend/components/PkpAvatarInitials/pkpAvatarInitials.vue

jardakotesovec · 2025-08-20T07:24:58Z

src/frontend/components/PkpButton/PkpButton.vue

@@ -13,12 +13,15 @@ const props = defineProps({
 		type: Boolean,
 		default: false,
 	},
+	/** Use when this button represents an action such as delete, go back, revert or cancel. */
+	isWarnable: Boolean,


I will confirm this one with Devika.. maybe we can have just primary and secondary. Goal here will be again just have as least variants as possible.. so its easier to create styling across the themes.

Any updates on this?

Most likely we will be fine just using the primary&secondary styling. So lets stick with that for now. Primary would be used for delete button, and secondary for cancel.

jardakotesovec · 2025-08-20T07:56:31Z

src/frontend/components/PkpModal/PkpDialog.vue

@@ -68,6 +68,9 @@ import {
 } from 'reka-ui';
 import PkpIcon from '@/frontend/components/PkpIcon/PkpIcon.vue';

+import {useLocalize} from '@/composables/useLocalize';
+const {t} = useLocalize();


Originally I thought that we will just pass the labels via props. . thats pattern we used to do.

But good middle ground is probably just manually use setLocaleKeys as you did and than still use nice t in the code.

Thats why I have not included useLocalise. So lets include it.. but lets make copy and call it usePkpLocalise..

jardakotesovec · 2025-08-20T08:00:03Z

src/frontend/components/PkpUserComment/PkpUserCommentsList.vue

+
+		<div v-if="isLatestPublication && !!currentUser">
+			<PkpTextarea
+				placeholder="What do you think about this publication? Type your comments here."


Yes.. and also label is essential for accessibility.
So we need to pass the same text as label. Because screen readers are somewhat inconsistent how they present placeholder.. And add prop on PkpTextarea to hide it (only for screenreaders).

jardakotesovec · 2025-08-20T08:02:03Z

src/frontend/components/PkpUserComment/PkpUserCommentsList.vue

+import {usePkpModal} from '@/frontend/composables/usePkpModal';
+import {usePkpFetch} from '@/frontend/composables/usePkpFetch';
+import {useUrl} from '@/frontend/composables/usePkpUrl';
+import {useDate} from '@/composables/useDate';


I was hesitant to bring useDate as its quite sizeable.. But it would be good to have it.. so lets stick with it.. lets just make copy as well.

And I will iterate on it later to see whether I can make the dependency smaller.. or fine other way to do it.. but its good to have this abstracted in composable.

jardakotesovec · 2025-08-20T08:05:53Z

src/frontend/components/PkpUserComment/PkpUserCommentsList.vue

+	commentText.value = '';
+	// Add the new comment to the top of the comments list.
+	comments.value.unshift(data);
+}


Once we update something via API - its best to reload the data and consider that as source of the truth.

Doing optimistic update on top of it is fine to make it bit faster. But in general for simplicity - I basically just call API for update and than refresh the data. So here doing the same would be fine as well. As long as there is indication that the submit is being processed its fine to show it with bit of delay.

I agree, the simpler approach would be to refresh the data via API call.

However, I ran into a few concerns/roadblocks trying to refresh an entire list built from multiple paginated responses.

The challenge: when a user adds a new comment, it will be available on page 1 of the paginated results (most recent items comes first). To update the list via API to include the new comment, while preserving items already shown, I’d need to refetch items from page 1 up to the current page.

Options I’ve considered:

Refetch pages 1 to current

would work, but not performant. Also, the new item on page 1 pushes another off and we might lose an item after finish fetching because it was pushed to another page not in the range(pages 1 to current) fetched.

Just fetch for page 1 where the new comment is expected to be and update that portion of the list
there's possibility that moderators approve new comments during the same time span, and those could potentially end up on that list returned from API depending on when they were created, and potentially displacing the expected new comment

Single request with pageSize = current length of comments + 1 (+1 to compensate for the newly added comment)

More efficient, but there's possibility that moderators approve new comments during the same time span, and those could potentially end up on that list returned from API depending on when they were created, thus potentially pushing more items off the final updated list shown to user.

Single request with pageSize = current length + 1 full page size

Benefits of number 3, but reduces the potential of a newly approved comment displacing one that was already visible to the user -- this is still a possibility though.

I initially went for the optimistic update, appending the created comment returned from the API, due to the challenges outlined above along with the fact that a newly created comment is only visible to the creator, and we just want to display it to them to confirm creation and to inform them that it will need approval before being visible to others.

I'm probably thinking about this incorrectly, so I'm open to feedback on how to approach this

On further thought, I don't think newly approved comments displacing others is a real concern at this time, and approach 3 or 4 should suffice in updating the list.

I see.. did not think about the "show more" impact here. In general "show more" is better to implemented with "cursor based" pagination.. as that ensures that newly added message (from anyone) does not mess up the list. But not sure how much effort that would be.. as we don't expect significant use - it will be fine either way.

Regarding what to do after posting new message. I think just going back to refetching "first page" and throwing rest should be fine.. and user can use show more again if they want to go back to the older messages. Since new messages is being added on top - I don't think that would be problematic.

jardakotesovec · 2025-08-20T08:08:41Z

src/frontend/components/PkpUserComment/PkpUserCommentsList.vue

+	pageCount.value = pagination.value.pageCount;
+	showMoreCommentsCount.value =
+		showMoreCommentsCount.value - props.itemsPerPage;
+}


Looks good.. useFetchPaginated was not really created for "show more".. its done for using pagination.. so there would be opportunity to abstract that better.. but I think this is fine.

taslangraham · 2025-08-22T14:21:55Z

Thanks for the review @ewhanson & @jardakotesovec.

@jardakotesovec I've made update based on feedback given. Please take a look.

taslangraham mentioned this pull request Jun 28, 2025

Add front end UI for commenting in themes pkp/pkp-lib#11327

Open

6 tasks

taslangraham commented Jul 9, 2025

View reviewed changes

src/components/UserComment/UserComment.vue Outdated Show resolved Hide resolved

taslangraham commented Jul 9, 2025

View reviewed changes

src/components/UserComment/UserComment.vue Outdated Show resolved Hide resolved

taslangraham commented Jul 9, 2025

View reviewed changes

src/components/UserComment/UserComment.vue Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

taslangraham force-pushed the i11327-main branch from 30c85fe to 238b189 Compare August 14, 2025 03:55

taslangraham added 4 commits August 14, 2025 14:56

WIP: pkp/pkp-lib#11327 Implement public facing UI for user comments

81839f7

pkp/pkp-lib#11698 Add additional icons and components to frontend UI

3efa589

pkp/pkp-lib#11327 Implement public comments UI

3358d82

pkp/pkp-lib#11327 Allow user to delete comment

234a0ef

taslangraham force-pushed the i11327-main branch 2 times, most recently from 2f3351f to 1f2de49 Compare August 14, 2025 20:49

taslangraham changed the title ~~WIP: pkp/pkp-lib#11327 Implement public facing UI for user comments~~ pkp/pkp-lib#11327 Implement public facing UI for user comments Aug 14, 2025

taslangraham force-pushed the i11327-main branch from 1f2de49 to c528830 Compare August 14, 2025 21:09

ewhanson reviewed Aug 19, 2025

View reviewed changes

jardakotesovec requested changes Aug 20, 2025

View reviewed changes

taslangraham marked this pull request as ready for review August 20, 2025 15:50

taslangraham force-pushed the i11327-main branch 5 times, most recently from 67ae0c2 to 2b21f3f Compare August 22, 2025 13:29

pkp/pkp-lib#11327 Code cleanup and refactoring

39adda2

taslangraham force-pushed the i11327-main branch from 2b21f3f to 39adda2 Compare August 22, 2025 16:00

pkp/pkp-lib#11327 Implement public facing UI for user comments #659

Are you sure you want to change the base?

pkp/pkp-lib#11327 Implement public facing UI for user comments #659

Conversation

taslangraham commented Jul 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

ewhanson 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!

jardakotesovec left a comment

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!

jardakotesovec Aug 22, 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!

Choose a reason for hiding this comment

Uh oh!

taslangraham Aug 21, 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!

taslangraham commented Aug 22, 2025

Uh oh!

Uh oh!

jardakotesovec Aug 22, 2025 •

edited

Loading

taslangraham Aug 21, 2025 •

edited

Loading