From 364ad95d3cedafac0b32e1510fc8fddfa2557cf6 Mon Sep 17 00:00:00 2001 From: feruzm Date: Sat, 1 Aug 2026 08:07:15 +0000 Subject: [PATCH 1/2] fix(post): deleting a comment from the options sheet no longer pops the screen postComments mounted the sheet for comments without an onDelete, so the sheet's own delete path ran: it called navigation.goBack() and left the post the user was reading, and skipped _handleDeleteComment, which the same screen already passes to the inline delete button and which owns the error extraction and deleted-key cache work. Passes onDelete forwarding to that handler, with the same arguments the inline button uses. Also makes the fallback safe rather than relying on every consumer remembering. Going back only makes sense when the deleted content is the screen, which is true for a root post and false for a comment in a list, so the fallback now pops only for content with no parent. The existing comment on _deletePost already warned about this and it was still missed twice while adding consumers. Audited every PostOptionsModal consumer. postScreen and editorScreen delete the content that is the screen, so the pop is correct there. postsListContainer has the same gap for posts in a feed and no list removal to delegate to; filed as #3407 rather than half-fixed here, since suppressing the pop without removal would leave a stale feed with no feedback. --- .../postComments/container/postComments.tsx | 22 ++++++++++++++++++- .../container/postOptionsModal.tsx | 10 ++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/components/postComments/container/postComments.tsx b/src/components/postComments/container/postComments.tsx index 2a74af7fb3..d3b417c9ed 100644 --- a/src/components/postComments/container/postComments.tsx +++ b/src/components/postComments/container/postComments.tsx @@ -270,6 +270,22 @@ const PostComments = forwardRef( }); }, []); + // The sheet is opened for comments here, so its own delete path would call + // navigation.goBack() and leave the post the user is reading, and would skip + // the error handling and deleted-key cache work this screen already owns. + // Same arguments the inline delete button passes. + const _handleDeleteFromMenu = useCallback( + (comment) => + _handleDeleteComment( + comment.permlink, + comment.parent_permlink, + comment.parent_author, + comment.root_author, + comment.root_permlink, + ), + [_handleDeleteComment], + ); + const _handleShowOptionsMenu = useCallback((comment) => { if (postOptionsModalRef.current) { postOptionsModalRef.current.show(comment); @@ -454,7 +470,11 @@ const PostComments = forwardRef( overScrollMode="never" /> - + ); }, diff --git a/src/components/postOptionsModal/container/postOptionsModal.tsx b/src/components/postOptionsModal/container/postOptionsModal.tsx index 77dcafe6a4..c1e76f191d 100644 --- a/src/components/postOptionsModal/container/postOptionsModal.tsx +++ b/src/components/postOptionsModal/container/postOptionsModal.tsx @@ -478,7 +478,15 @@ const PostOptionsModal = ( parentAuthor: content.parent_author || '', parentPermlink: content.parent_permlink || '', }); - navigation.goBack(); + // Going back only makes sense when the deleted content *is* the screen, + // which is true for a root post and false for a comment in a list: + // there, popping navigates away from the profile or post the user was + // reading. Consumers that own the surrounding list should pass + // `onDelete` and handle removal themselves; this keeps the fallback + // from popping the wrong screen when they do not. + if (!content?.parent_author) { + navigation.goBack(); + } dispatch( toastNotification( intl.formatMessage({ From f12935f28f5ceb55631293a6a5ba546259722a4a Mon Sep 17 00:00:00 2001 From: feruzm Date: Sat, 1 Aug 2026 09:27:40 +0000 Subject: [PATCH 2/2] fix(post): drop the parent_author pop heuristic and the double confirmation Two review findings, both regressions in the previous commit. postScreen renders comments and waves as primary content, so gating navigation.goBack() on parent_author left a comment's own detail screen showing content that had just been deleted. Content shape cannot distinguish 'comment in a list' from 'comment as the screen' - only the caller knows - so the fallback pops unconditionally again and consumers that own a list must pass onDelete. #3407 tracks inverting this into an explicit opt-in, which is the version that fails safe. The delete delegation also asked twice: PostOptionsModal confirms before invoking onDelete, and _handleDeleteComment then confirmed again, so one action needed two confirmations and could be cancelled at the second after being confirmed at the first. Splits that handler in two. _deleteCommentConfirmed mutates without prompting and is what the sheet delegates to; _handleDeleteComment prompts and then calls it, for the inline delete button which has no confirmation of its own. commentsContainer has no confirmation step, so the equivalent path added in #3405 was never affected. --- .../postComments/container/postComments.tsx | 115 ++++++++++-------- .../container/postOptionsModal.tsx | 18 +-- 2 files changed, 72 insertions(+), 61 deletions(-) diff --git a/src/components/postComments/container/postComments.tsx b/src/components/postComments/container/postComments.tsx index d3b417c9ed..c03be259bb 100644 --- a/src/components/postComments/container/postComments.tsx +++ b/src/components/postComments/container/postComments.tsx @@ -171,59 +171,64 @@ const PostComments = forwardRef( [navigation], ); - const _handleDeleteComment = useCallback( + // Mutation only, no confirmation. The options sheet confirms before invoking + // its onDelete, so a handler that prompts again would ask twice for one + // action, and let the user cancel the second after confirming the first. + const _deleteCommentConfirmed = useCallback( async (_permlink, _parentPermlink?, _parentAuthor?, _rootAuthor?, _rootPermlink?) => { - const _onConfirmDelete = async () => { - const deletedKey = `${currentAccountName}/${_permlink}`; - const extractErrorDetail = (error: any) => { - const detail = - error?.message || - error?.response?.message || - error?.response?.data?.message || - error?.data?.message || - error?.error_description || - error?.jse_shortmsg; - return typeof detail === 'string' ? detail : JSON.stringify(error); - }; - - setHiddenCommentKeys((prev) => { - const next = new Set(prev); - next.add(deletedKey); - return next; - }); + const deletedKey = `${currentAccountName}/${_permlink}`; + const extractErrorDetail = (error: any) => { + const detail = + error?.message || + error?.response?.message || + error?.response?.data?.message || + error?.data?.message || + error?.error_description || + error?.jse_shortmsg; + return typeof detail === 'string' ? detail : JSON.stringify(error); + }; + + setHiddenCommentKeys((prev) => { + const next = new Set(prev); + next.add(deletedKey); + return next; + }); - try { - await deleteComment({ - author: currentAccountName, - permlink: _permlink, - parentAuthor: _parentAuthor, - parentPermlink: _parentPermlink || permlink, - rootAuthor: _rootAuthor || author, - rootPermlink: _rootPermlink || permlink, + try { + await deleteComment({ + author: currentAccountName, + permlink: _permlink, + parentAuthor: _parentAuthor, + parentPermlink: _parentPermlink || permlink, + rootAuthor: _rootAuthor || author, + rootPermlink: _rootPermlink || permlink, + }); + console.log('deleted comment', `${currentAccountName}/${_permlink}`); + } catch (err) { + const stillExists = !!discussionQuery.data?.[deletedKey]; + if (stillExists) { + setHiddenCommentKeys((prev) => { + const next = new Set(prev); + next.delete(deletedKey); + return next; }); - console.log('deleted comment', `${currentAccountName}/${_permlink}`); - } catch (err) { - const stillExists = !!discussionQuery.data?.[deletedKey]; - if (stillExists) { - setHiddenCommentKeys((prev) => { - const next = new Set(prev); - next.delete(deletedKey); - return next; - }); - } - const errorDetail = extractErrorDetail(err); - if (stillExists) { - dispatch(toastNotification(`Failed to delete comment: ${errorDetail}`)); - } else { - console.log( - 'delete returned error but comment is already absent in cache', - deletedKey, - ); - } - console.warn('Failed to delete comment', err); } - }; + const errorDetail = extractErrorDetail(err); + if (stillExists) { + dispatch(toastNotification(`Failed to delete comment: ${errorDetail}`)); + } else { + console.log('delete returned error but comment is already absent in cache', deletedKey); + } + console.warn('Failed to delete comment', err); + } + }, + [author, currentAccountName, deleteComment, dispatch, discussionQuery.data, intl, permlink], + ); + // Confirms, then mutates. Used by the inline delete button, which has no + // confirmation of its own. + const _handleDeleteComment = useCallback( + async (_permlink, _parentPermlink?, _parentAuthor?, _rootAuthor?, _rootPermlink?) => { const action = await SheetManager.show(SheetNames.ACTION_MODAL, { payload: { title: intl.formatMessage({ id: 'delete.confirm_delete_title' }), @@ -241,10 +246,16 @@ const PostComments = forwardRef( }); if (action === 'confirm') { - _onConfirmDelete(); + _deleteCommentConfirmed( + _permlink, + _parentPermlink, + _parentAuthor, + _rootAuthor, + _rootPermlink, + ); } }, - [author, currentAccountName, deleteComment, dispatch, discussionQuery.data, intl, permlink], + [_deleteCommentConfirmed, intl], ); const _openReplyThread = useCallback( @@ -276,14 +287,14 @@ const PostComments = forwardRef( // Same arguments the inline delete button passes. const _handleDeleteFromMenu = useCallback( (comment) => - _handleDeleteComment( + _deleteCommentConfirmed( comment.permlink, comment.parent_permlink, comment.parent_author, comment.root_author, comment.root_permlink, ), - [_handleDeleteComment], + [_deleteCommentConfirmed], ); const _handleShowOptionsMenu = useCallback((comment) => { diff --git a/src/components/postOptionsModal/container/postOptionsModal.tsx b/src/components/postOptionsModal/container/postOptionsModal.tsx index c1e76f191d..a5fba02c7d 100644 --- a/src/components/postOptionsModal/container/postOptionsModal.tsx +++ b/src/components/postOptionsModal/container/postOptionsModal.tsx @@ -478,15 +478,15 @@ const PostOptionsModal = ( parentAuthor: content.parent_author || '', parentPermlink: content.parent_permlink || '', }); - // Going back only makes sense when the deleted content *is* the screen, - // which is true for a root post and false for a comment in a list: - // there, popping navigates away from the profile or post the user was - // reading. Consumers that own the surrounding list should pass - // `onDelete` and handle removal themselves; this keeps the fallback - // from popping the wrong screen when they do not. - if (!content?.parent_author) { - navigation.goBack(); - } + // Always pops, because only the caller knows whether the deleted content + // *is* the screen. postScreen renders comments and waves as primary + // content too, so `parent_author` cannot stand in for that: gating on it + // left a comment's own detail screen showing deleted content. + // + // Consumers that own a surrounding list must therefore pass `onDelete` + // and handle removal themselves. See #3407 for inverting this into an + // explicit opt-in, which fails safe in the other direction. + navigation.goBack(); dispatch( toastNotification( intl.formatMessage({