0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-01-20 22:42:53 -05:00

Cleaned up Comment component (#8)

refs https://github.com/TryGhost/Team/issues/1858
fixes https://github.com/TryGhost/Team/issues/1789

- Split up the Comment component in many small Components to make it easier to read and maintain
- Added support for synchronous actions, which are required for actions that affect the context state based on the current value (e.g., increasing a value by one) because of the asynchronous nature of setState:

Before this change
```
// Context state: {hello: 0};
dispatchAction('increaseHelloByOne')
dispatchAction('increaseHelloByOne')
```

Could end up having a state `{hello: 1}` instead of the expected `{hello: 2}`, because underlying this would resolve into:

```
// Context state: {hello: 0};
const hello = {hello: 0};
setState({hello: hello + 1});
setState({hello: hello + 1});
```

Instead of

```
// Context state: {hello: 0};
setState(({hello}) => {hello: hello + 1});
setState(({hello}) => {hello: hello + 1});
```

Synchronous actions now support this.

- Removed deprecated `onAction` context state function
- Replaced the boolean based form checking by the more reliable counter based checking that uses synchronous actions (reason we needed synchronous actions) (fixes https://github.com/TryGhost/Team/issues/1789)
- Prevent creating a new `dispatchAction` function every time the context state changes, by using bind. This prevents infinte updates in `useEffect` hooks that depend on `dispatchAction` and also update the context via actions.
This commit is contained in:
Simon Backx 2022-08-30 16:25:40 +02:00 committed by GitHub
parent 3a29a55228
commit b701ba9c0d
8 changed files with 331 additions and 160 deletions

View file

@ -1,6 +1,6 @@
import Frame from './components/Frame';
import React from 'react';
import ActionHandler from './actions';
import {isSyncAction, ActionHandler, SyncActionHandler} from './actions';
import {createPopupNotification} from './utils/helpers';
import AppContext from './AppContext';
import {hasMode} from './utils/check-mode';
@ -51,10 +51,14 @@ export default class App extends React.Component {
customSiteUrl: props.customSiteUrl,
postId: props.postId,
popup: null,
accentColor: props.accentColor
accentColor: props.accentColor,
secundaryFormCount: 0
};
this.adminApi = null;
this.GhostApi = null;
// Bind to make sure we have a variable reference (and don't need to create a new binded function in our context value every time the state changes)
this.dispatchAction = this.dispatchAction.bind(this);
}
/** Initialize comments setup on load, fetch data and setup state*/
@ -111,6 +115,15 @@ export default class App extends React.Component {
/** Handle actions from across App and update App state */
async dispatchAction(action, data) {
if (isSyncAction(action)) {
// Makes sure we correctly handle the old state
// because updates to state may be asynchronous
// so calling dispatchAction('counterUp') multiple times, may yield unexpected results if we don't use a callback function
this.setState((state) => {
return SyncActionHandler({action, data, state, api: this.GhostApi, adminApi: this.adminApi});
});
return;
}
clearTimeout(this.timeoutId);
this.setState({
action: `${action}:running`
@ -252,7 +265,7 @@ export default class App extends React.Component {
/**Get final App level context from App state*/
getContextFromState() {
const {action, popupNotification, customSiteUrl, member, comments, pagination, commentCount, postId, admin, popup} = this.state;
const {action, popupNotification, customSiteUrl, member, comments, pagination, commentCount, postId, admin, popup, secundaryFormCount} = this.state;
return {
action,
popupNotification,
@ -272,14 +285,12 @@ export default class App extends React.Component {
appVersion: this.props.appVersion,
stylesUrl: this.props.stylesUrl,
publication: this.props.publication,
secundaryFormCount: secundaryFormCount,
popup,
dispatchAction: (_action, data) => this.dispatchAction(_action, data),
/**
* @deprecated
* Use dispatchAction instead
*/
onAction: (_action, data) => this.dispatchAction(_action, data)
// Warning: make sure we pass a variable here (binded in constructor), because if we create a new function here, it will also change when anything in the state changes
// causing loops in useEffect hooks that depend on dispatchAction
dispatchAction: this.dispatchAction
};
}

View file

@ -1,15 +1,6 @@
// Ref: https://reactjs.org/docs/context.html
const React = require('react');
const AppContext = React.createContext({
site: {},
member: {},
action: '',
brandColor: '',
pageData: {},
onAction: (action, data) => {
return {action, data};
}
});
const AppContext = React.createContext({});
export default AppContext;

View file

@ -332,6 +332,26 @@ function closePopup() {
};
}
function increaseSecundaryFormCount({state}) {
return {
secundaryFormCount: state.secundaryFormCount + 1
};
}
function decreaseSecundaryFormCount({state}) {
return {
secundaryFormCount: state.secundaryFormCount - 1
};
}
// Sync actions make use of setState((currentState) => newState), to avoid 'race' conditions
const SyncActions = {
openPopup,
closePopup,
increaseSecundaryFormCount,
decreaseSecundaryFormCount
};
const Actions = {
// Put your actions here
addComment,
@ -345,16 +365,28 @@ const Actions = {
addReply,
loadMoreComments,
loadMoreReplies,
updateMember,
openPopup,
closePopup
updateMember
};
export function isSyncAction(action) {
return !!SyncActions[action];
}
/** Handle actions in the App, returns updated state */
export default async function ActionHandler({action, data, state, api, adminApi}) {
export async function ActionHandler({action, data, state, api, adminApi}) {
const handler = Actions[action];
if (handler) {
return await handler({data, state, api, adminApi}) || {};
}
return {};
}
/** Handle actions in the App, returns updated state */
export function SyncActionHandler({action, data, state, api, adminApi}) {
const handler = SyncActions[action];
if (handler) {
// Do not await here
return handler({data, state, api, adminApi}) || {};
}
return {};
}

View file

@ -3,9 +3,23 @@ import AppContext from '../AppContext';
import {getInitials} from '../utils/helpers';
import {ReactComponent as AvatarIcon} from '../images/icons/avatar.svg';
const Avatar = (props) => {
const {member} = useContext(AppContext);
const memberName = member?.name ?? props.comment?.member?.name;
const Avatar = ({comment, size, isBlank}) => {
const {member, avatarSaturation} = useContext(AppContext);
const dimensionClasses = (size === 'small' ? 'w-6 h-6 sm:w-8 sm:h-8' : 'w-9 h-9 sm:w-[40px] sm:h-[40px]');
// When an avatar has been deleted or hidden
// todo: move to seperate component
if (isBlank) {
return (
<figure className={`relative ${dimensionClasses}`}>
<div className={`flex items-center justify-center rounded-full bg-[rgba(200,200,200,0.3)] ${dimensionClasses}`}>
<AvatarIcon className="stroke-white dark:opacity-70" />
</div>
</figure>
);
}
const memberName = member?.name ?? comment?.member?.name;
const getHashOfString = (str) => {
let hash = 0;
@ -21,13 +35,13 @@ const Avatar = (props) => {
};
const generateHSL = () => {
let commentMember = (props.comment ? props.comment.member : member);
let commentMember = (comment ? comment.member : member);
if (!commentMember || !commentMember.name) {
return [0,0,10];
}
const saturation = isNaN(props.saturation) ? 50 : props.saturation;
const saturation = isNaN(avatarSaturation) ? 50 : avatarSaturation;
const hRange = [0, 360];
const lRangeTop = Math.round(saturation / (100 / 30)) + 30;
@ -46,11 +60,11 @@ const Avatar = (props) => {
};
const commentGetInitials = () => {
if (props.comment && !props.comment.member) {
if (comment && !comment.member) {
return getInitials('Deleted member');
}
let commentMember = (props.comment ? props.comment.member : member);
let commentMember = (comment ? comment.member : member);
if (!commentMember || !commentMember.name) {
return getInitials('Anonymous');
@ -58,9 +72,8 @@ const Avatar = (props) => {
return getInitials(commentMember.name);
};
let dimensionClasses = (props.size === 'small' ? 'w-6 h-6 sm:w-8 sm:h-8' : 'w-9 h-9 sm:w-[40px] sm:h-[40px]');
let initialsClasses = (props.size === 'small' ? 'text-sm' : 'text-lg');
let commentMember = (props.comment ? props.comment.member : member);
let initialsClasses = (size === 'small' ? 'text-sm' : 'text-lg');
let commentMember = (comment ? comment.member : member);
const bgColor = HSLtoString(generateHSL());
const avatarStyle = {
@ -80,15 +93,6 @@ const Avatar = (props) => {
</>
);
// When an avatar has been deleted or hidden
if (props.isBlank) {
avatarEl = (
<div className={`flex items-center justify-center rounded-full bg-[rgba(200,200,200,0.3)] ${dimensionClasses}`}>
<AvatarIcon className="stroke-white dark:opacity-70" />
</div>
);
}
return (
<figure className={`relative ${dimensionClasses}`}>
{avatarEl}

View file

@ -1,4 +1,4 @@
import React, {useContext, useEffect, useState} from 'react';
import React, {useContext, useState} from 'react';
import {Transition} from '@headlessui/react';
import Avatar from './Avatar';
import Like from './Like';
@ -7,39 +7,58 @@ import More from './More';
import Form from './Form';
import Replies from './Replies';
import AppContext from '../AppContext';
import {formatRelativeTime, formatExplicitTime} from '../utils/helpers';
import {formatRelativeTime, formatExplicitTime, isCommentPublished} from '../utils/helpers';
function EditedInfo({comment}) {
if (!comment.edited_at) {
return null;
}
function AnimatedComment({comment, parent}) {
return (
<span>
<span className="mx-[0.3em]">·</span>Edited
</span>
<Transition
appear
show={true}
enter="transition-opacity duration-300 ease-out"
enterFrom="opacity-0"
enterTo="opacity-100"
leave="transition-opacity duration-100"
leaveFrom="opacity-100"
leaveTo="opacity-0"
>
<EditableComment comment={comment} parent={parent} />
</Transition>
);
}
const Comment = ({updateIsEditing = null, isEditing, ...props}) => {
function EditableComment({comment, parent}) {
const [isInEditMode, setIsInEditMode] = useState(false);
const [isInReplyMode, setIsInReplyMode] = useState(false);
const {admin, avatarSaturation, member, commentsEnabled, dispatchAction} = useContext(AppContext);
let comment = props.comment;
useEffect(() => {
// This doesn't work, and should receive an update.
// When one Comment shows reply, while a different Form hides the reply form at the same time, the global
// 'isEditing' is unreliable. We should use a counter of total open forms instead of a boolean.
updateIsEditing?.(isInReplyMode || isInEditMode);
}, [updateIsEditing, isInReplyMode, isInEditMode]);
const toggleEditMode = () => {
setIsInEditMode(current => !current);
};
const closeEditMode = () => {
setIsInEditMode(false);
};
const openEditMode = () => {
setIsInEditMode(true);
};
if (isInEditMode) {
return (
<EditForm comment={comment} close={closeEditMode} parent={parent} />
);
} else {
return (<Comment comment={comment} openEditMode={openEditMode} parent={parent} />);
}
}
function Comment({comment, parent, openEditMode}) {
const isPublished = isCommentPublished(comment);
if (isPublished) {
return (<PublishedComment comment={comment} parent={parent} openEditMode={openEditMode} />);
}
return (<UnpublishedComment comment={comment} openEditMode={openEditMode} />);
}
function PublishedComment({comment, parent, openEditMode}) {
const [isInReplyMode, setIsInReplyMode] = useState(false);
const {dispatchAction} = useContext(AppContext);
const toggleReplyMode = async () => {
if (!isInReplyMode) {
// First load all the replies before opening the reply model
@ -52,100 +71,196 @@ const Comment = ({updateIsEditing = null, isEditing, ...props}) => {
setIsInReplyMode(false);
};
const hasReplies = comment.replies && comment.replies.length > 0;
const isNotPublished = comment.status !== 'published';
const html = {__html: comment.html};
const hasReplies = isInReplyMode || (comment.replies && comment.replies.length > 0);
const avatar = (<Avatar comment={comment} />);
return (
<CommentLayout hasReplies={hasReplies} avatar={avatar}>
<CommentHeader comment={comment} />
<CommentBody html={comment.html} />
<CommentMenu comment={comment} parent={parent} isInReplyMode={isInReplyMode} toggleReplyMode={toggleReplyMode} openEditMode={openEditMode} />
<RepliesContainer comment={comment} />
<ReplyForm comment={comment} isInReplyMode={isInReplyMode} closeReplyMode={closeReplyMode} />
</CommentLayout>
);
}
function UnpublishedComment({comment, openEditMode}) {
const {admin} = useContext(AppContext);
let notPublishedMessage;
if (isNotPublished) {
if (admin && comment.status === 'hidden') {
notPublishedMessage = 'This comment has been hidden.';
} else {
notPublishedMessage = 'This comment has been removed.';
}
if (admin && comment.status === 'hidden') {
notPublishedMessage = 'This comment has been hidden.';
} else {
notPublishedMessage = 'This comment has been removed.';
}
// TODO: consider swapping this with a seperate avatar component
const blankAvatar = (<Avatar isBlank={true} />);
const hasReplies = comment.replies && comment.replies.length > 0;
return (
<CommentLayout hasReplies={hasReplies} avatar={blankAvatar}>
<div className="-mt-[3px] mb-2 flex items-start">
<div className="flex h-12 flex-row items-center gap-4 pb-[8px] pr-4">
<p className="mt-[4px] font-sans text-[16px] italic leading-normal text-neutral-300 dark:text-[rgba(255,255,255,0.5)]">{notPublishedMessage}</p>
<div className="mt-[4px]">
<More comment={comment} toggleEdit={openEditMode} />
</div>
</div>
</div>
<RepliesContainer comment={comment} />
</CommentLayout>
);
}
// Helper components
function MemberBio({comment}) {
const {member} = useContext(AppContext);
const memberBio = member && comment.member && comment.member.uuid === member.uuid ? member.bio : comment?.member?.bio;
if (!memberBio) {
return null;
}
return (
<span>{memberBio}<span className="mx-[0.3em]">·</span></span>
);
}
function EditedInfo({comment}) {
if (!comment.edited_at) {
return null;
}
return (
<span>
<span className="mx-[0.3em]">·</span>Edited
</span>
);
}
function RepliesContainer({comment}) {
const hasReplies = comment.replies && comment.replies.length > 0;
if (!hasReplies) {
return null;
}
return (
<div className="mt-10 mb-4 sm:mb-0">
<Replies comment={comment} />
</div>
);
}
function ReplyForm({comment, isInReplyMode, closeReplyMode}) {
if (!isInReplyMode) {
return null;
}
return (
<div className="my-10">
<Form parent={comment} close={closeReplyMode} isReply={true} />
</div>
);
}
function EditForm({comment, parent, close}) {
return (
<Form comment={comment} close={close} parent={parent} isEdit={true} />
);
}
//
// -- Published comment components --
//
// TODO: move name detection to helper
function AuthorName({comment}) {
const name = !comment.member ? 'Deleted member' : (comment.member.name ? comment.member.name : 'Anonymous');
return (
<h4 className="text-[rgb(23,23,23] font-sans text-[17px] font-bold tracking-tight dark:text-[rgba(255,255,255,0.85)]">
{name}
</h4>
);
}
function CommentHeader({comment}) {
return (
<div className="-mt-[3px] mb-2 flex items-start">
<div>
<AuthorName comment={comment} />
<div className="flex items-baseline pr-4 font-sans text-[14px] tracking-tight text-neutral-400 dark:text-[rgba(255,255,255,0.5)]">
<span>
<MemberBio comment={comment}/>
<span title={formatExplicitTime(comment.created_at)}>{formatRelativeTime(comment.created_at)}</span>
<EditedInfo comment={comment} />
</span>
</div>
</div>
</div>
);
}
function CommentBody({html}) {
const dangerouslySetInnerHTML = {__html: html};
return (
<div className="mt mb-2 flex flex-row items-center gap-4 pr-4">
<p dangerouslySetInnerHTML={dangerouslySetInnerHTML} className="gh-comment-content font-sans text-[16px] leading-normal text-neutral-900 dark:text-[rgba(255,255,255,0.85)]" data-testid="comment-content"/>
</div>
);
}
function CommentMenu({comment, toggleReplyMode, isInReplyMode, openEditMode, parent}) {
// If this comment is from the current member, always override member
// with the member from the context, so we update the bio in existing comments when we change it
const {member, commentsEnabled} = useContext(AppContext);
const paidOnly = commentsEnabled === 'paid';
const isPaidMember = member && !!member.paid;
const canReply = member && (isPaidMember || !paidOnly);
const canReply = member && (isPaidMember || !paidOnly) && !parent;
// If this comment is from the current member, always override member
// with the member from the context, so we update the bio in existing comments when we change it
const memberBio = member && comment.member && comment.member.uuid === member.uuid ? member.bio : comment?.member?.bio;
return (
<div className="flex items-center gap-5">
{<Like comment={comment} />}
{(canReply && <Reply comment={comment} toggleReply={toggleReplyMode} isReplying={isInReplyMode} />)}
{<More comment={comment} toggleEdit={openEditMode} />}
</div>
);
}
if (isInEditMode) {
return (
<Form comment={comment} close={closeEditMode} parent={props.parent} isEdit={true} />
);
} else {
return (
<Transition
appear
show={true}
enter="transition-opacity duration-300 ease-out"
enterFrom="opacity-0"
enterTo="opacity-100"
leave="transition-opacity duration-100"
leaveFrom="opacity-100"
leaveTo="opacity-0"
>
<div className={`flex w-full flex-row ${hasReplies ? 'mb-0' : 'mb-10'}`} data-testid="comment-component">
<div className="mr-3 flex flex-col items-center justify-start">
<div className="flex-0 mb-4">
<Avatar comment={comment} saturation={avatarSaturation} isBlank={isNotPublished} />
</div>
{((!props.isReply && hasReplies) || isInReplyMode) && <div className="mb-2 h-full w-[3px] grow rounded bg-gradient-to-b from-neutral-100 via-neutral-100 to-transparent dark:from-[rgba(255,255,255,0.05)] dark:via-[rgba(255,255,255,0.05)]" />}
</div>
<div className="grow">
<div className="-mt-[3px] mb-2 flex items-start">
{isNotPublished ?
<div className="flex h-12 flex-row items-center gap-4 pb-[8px] pr-4">
<p className="mt-[4px] font-sans text-[16px] italic leading-normal text-neutral-300 dark:text-[rgba(255,255,255,0.5)]">{notPublishedMessage}</p>
<div className="mt-[4px]">
<More comment={comment} toggleEdit={toggleEditMode} />
</div>
</div> :
<div>
<h4 className="text-[rgb(23,23,23] font-sans text-[17px] font-bold tracking-tight dark:text-[rgba(255,255,255,0.85)]">{!comment.member ? 'Deleted member' : (comment.member.name ? comment.member.name : 'Anonymous')}</h4>
<div className="flex items-baseline pr-4 font-sans text-[14px] tracking-tight text-neutral-400 dark:text-[rgba(255,255,255,0.5)]">
<span>
{memberBio && <span>{memberBio}<span className="mx-[0.3em]">·</span></span>}
<span title={formatExplicitTime(comment.created_at)}>{formatRelativeTime(comment.created_at)}</span>
<EditedInfo comment={comment} />
</span>
</div>
</div>}
</div>
//
// -- Layout --
//
{!isNotPublished &&
<div className="mt mb-2 flex flex-row items-center gap-4 pr-4">
<p dangerouslySetInnerHTML={html} className="gh-comment-content font-sans text-[16px] leading-normal text-neutral-900 dark:text-[rgba(255,255,255,0.85)]" data-testid="comment-content"/>
</div>}
{!isNotPublished && (
<div className="flex items-center gap-5">
{<Like comment={comment} />}
{(canReply && (isNotPublished || !props.parent) && <Reply comment={comment} toggleReply={toggleReplyMode} isReplying={isInReplyMode} />)}
{<More comment={comment} toggleEdit={toggleEditMode} />}
</div>
)}
{hasReplies &&
<div className="mt-10 mb-4 sm:mb-0">
<Replies comment={comment} />
</div>
}
{isInReplyMode &&
<div className="my-10">
<Form parent={comment} close={closeReplyMode} isReply={true} />
</div>
}
</div>
</div>
</Transition>
);
function RepliesLine({hasReplies}) {
if (!hasReplies) {
return null;
}
};
export default Comment;
return (<div className="mb-2 h-full w-[3px] grow rounded bg-gradient-to-b from-neutral-100 via-neutral-100 to-transparent dark:from-[rgba(255,255,255,0.05)] dark:via-[rgba(255,255,255,0.05)]" />);
}
function CommentLayout({children, avatar, hasReplies}) {
return (
<div className={`flex w-full flex-row ${hasReplies === true ? 'mb-0' : 'mb-10'}`} data-testid="comment-component">
<div className="mr-3 flex flex-col items-center justify-start">
<div className="flex-0 mb-4">
{avatar}
</div>
<RepliesLine hasReplies={hasReplies} />
</div>
<div className="grow">
{children}
</div>
</div>
);
}
//
// -- Default --
//
export default AnimatedComment;

View file

@ -1,4 +1,4 @@
import React, {useContext, useEffect, useState} from 'react';
import React, {useContext, useEffect} from 'react';
import AppContext from '../AppContext';
import CTABox from './CTABox';
import Form from './Form';
@ -50,11 +50,8 @@ const CommentsBoxTitle = ({title, showCount, count}) => {
};
const CommentsBoxContent = (props) => {
// @todo: This doesn't work and should get replaced with a counter of total open forms. If total open forms > 0, don't show the main form.
const [isEditing, setIsEditing] = useState(false);
const {pagination, member, comments, commentCount, commentsEnabled, title, showCount} = useContext(AppContext);
const commentsElements = comments.slice().reverse().map(comment => <Comment isEditing={isEditing} comment={comment} key={comment.id} updateIsEditing={setIsEditing} />);
const {pagination, member, comments, commentCount, commentsEnabled, title, showCount, secundaryFormCount} = useContext(AppContext);
const commentsElements = comments.slice().reverse().map(comment => <Comment comment={comment} key={comment.id} />);
const paidOnly = commentsEnabled === 'paid';
const isPaidMember = member && !!member.paid;
@ -77,6 +74,8 @@ const CommentsBoxContent = (props) => {
}
}, []);
const hasOpenSecundaryForms = secundaryFormCount > 0;
return (
<>
<CommentsBoxTitle title={title} showCount={showCount} count={commentCount}/>
@ -85,7 +84,8 @@ const CommentsBoxContent = (props) => {
{commentsElements}
</div>
<div>
{ !isEditing
{secundaryFormCount}
{!hasOpenSecundaryForms
? (member ? (isPaidMember || !paidOnly ? <Form commentsCount={commentCount} /> : <CTABox isFirst={pagination?.total === 0} isPaid={paidOnly} />) : <CTABox isFirst={pagination?.total === 0} isPaid={paidOnly} />)
: null
}

View file

@ -12,7 +12,7 @@ import {GlobalEventBus} from '../utils/event-bus';
let formId = 0;
const Form = (props) => {
const {member, postId, dispatchAction, avatarSaturation} = useContext(AppContext);
const {member, postId, dispatchAction} = useContext(AppContext);
const [isFormOpen, setFormOpen] = useState(props.isReply || props.isEdit ? true : false);
const formEl = useRef(null);
const [progress, setProgress] = useState('default');
@ -24,6 +24,20 @@ const Form = (props) => {
const memberName = member?.name ?? comment?.member?.name;
const memberBio = member?.bio ?? comment?.member?.bio;
// Keep track of the amount of open forms
useEffect(() => {
if (!props.isEdit && !props.isReply) {
// General form
return;
}
dispatchAction('increaseSecundaryFormCount');
return () => {
dispatchAction('decreaseSecundaryFormCount');
};
}, [dispatchAction, props.isEdit, props.isReply]);
let buttonIcon = null;
if (progress === 'sending') {
buttonIcon = <SpinnerIcon className="h-[24px] w-[24px] fill-white dark:fill-black" />;
@ -437,7 +451,7 @@ const Form = (props) => {
</div>
<div className='absolute top-1 left-0 flex h-12 w-full items-center justify-start'>
<div className="mr-3 grow-0">
<Avatar comment={comment} saturation={avatarSaturation} className="pointer-events-none" />
<Avatar comment={comment} className="pointer-events-none" />
</div>
<div className="grow-1 w-full">
<Transition

View file

@ -114,3 +114,7 @@ export function getInitials(name) {
export function isMobile() {
return (Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0) < 480);
}
export function isCommentPublished(comment) {
return comment.status === 'published';
}