Skip to content

Commit af7cd28

Browse files
lyzadangerlyzadanger
committed
Refactor reply-toggling logic for top-level threads
Refactor the way headless threads are handled regarding visibility and reply toggling. Add an `AnnotationMissing` component; harmonize props between `Annotation` and `AnnotationMissing`. Add a new `AnnotationReplyToggle` component.
1 parent 4646070 commit af7cd28

File tree

9 files changed

+399
-95
lines changed

9 files changed

+399
-95
lines changed

‎src/sidebar/components/Annotation.js‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@ import { createElement } from 'preact';
33
import propTypes from 'prop-types';
44

55
import { useStoreProxy } from '../store/use-store';
6-
import { isReply, quote } from '../helpers/annotation-metadata';
6+
import { quote } from '../helpers/annotation-metadata';
77
import { withServices } from '../service-context';
88

99
import AnnotationActionBar from './AnnotationActionBar';
1010
import AnnotationBody from './AnnotationBody';
1111
import AnnotationEditor from './AnnotationEditor';
1212
import AnnotationHeader from './AnnotationHeader';
1313
import AnnotationQuote from './AnnotationQuote';
14-
import Button from './Button';
14+
import AnnotationReplyToggle from './AnnotationReplyToggle';
1515

1616
/**
1717
* @typedef {import("../../types/api").Annotation} Annotation
@@ -21,7 +21,10 @@ import Button from './Button';
2121
/**
2222
* @typedef AnnotationProps
2323
* @prop {Annotation} annotation
24-
* @prop {number} replyCount - Number of replies to this annotation (thread)
24+
* @prop {boolean} hasAppliedFilter - Is any filter applied currently?
25+
* @prop {boolean} isReply
26+
* @prop {VoidFunction} onToggleReplies - Callback to expand/collapse reply threads
27+
* @prop {number} replyCount - Number of replies to this annotation's thread
2528
* @prop {boolean} showDocumentInfo - Should extended document info be rendered (e.g. in non-sidebar contexts)?
2629
* @prop {boolean} threadIsCollapsed - Is the thread to which this annotation belongs currently collapsed?
2730
* @prop {Object} annotationsService - Injected service
@@ -34,10 +37,13 @@ import Button from './Button';
3437
*/
3538
function Annotation({
3639
annotation,
37-
annotationsService,
40+
hasAppliedFilter,
41+
isReply,
42+
onToggleReplies,
3843
replyCount,
3944
showDocumentInfo,
4045
threadIsCollapsed,
46+
annotationsService,
4147
}) {
4248
const store = useStoreProxy();
4349
const isFocused = store.isAnnotationFocused(annotation.$tag);
@@ -47,32 +53,21 @@ function Annotation({
4753
const userid = store.profile().userid;
4854
const isSaving = store.isSavingAnnotation(annotation);
4955

50-
const isCollapsedReply = isReply(annotation) && threadIsCollapsed;
56+
const isCollapsedReply = isReply && threadIsCollapsed;
5157

5258
const hasQuote = !!quote(annotation);
5359

5460
const isEditing = !!draft && !isSaving;
5561

56-
const toggleAction = threadIsCollapsed ? 'Show replies' : 'Hide replies';
57-
const toggleText = `${toggleAction} (${replyCount})`;
58-
59-
const shouldShowActions = !isSaving && !isEditing;
60-
const shouldShowReplyToggle = replyCount > 0 && !isReply(annotation);
62+
const showActions = !isSaving && !isEditing;
63+
const showReplyToggle = !isReply && !hasAppliedFilter && replyCount > 0;
6164

6265
const onReply = () => annotationsService.reply(annotation, userid);
6366

64-
const onToggleReplies = () =>
65-
// nb. We assume the annotation has an ID here because it is not possible
66-
// to create replies until the annotation has been saved.
67-
store.setExpanded(
68-
/** @type {string} */ (annotation.id),
69-
!!threadIsCollapsed
70-
);
71-
7267
return (
7368
<article
7469
className={classnames('Annotation', {
75-
'Annotation--reply': isReply(annotation),
70+
'Annotation--reply': isReply,
7671
'is-collapsed': threadIsCollapsed,
7772
'is-focused': isFocused,
7873
})}
@@ -98,15 +93,15 @@ function Annotation({
9893
{!isCollapsedReply && (
9994
<footer className="Annotation__footer">
10095
<div className="Annotation__controls u-layout-row">
101-
{shouldShowReplyToggle && (
102-
<Button
103-
className="Annotation__reply-toggle"
104-
onClick={onToggleReplies}
105-
buttonText={toggleText}
96+
{showReplyToggle && (
97+
<AnnotationReplyToggle
98+
onToggleReplies={onToggleReplies}
99+
replyCount={replyCount}
100+
threadIsCollapsed={threadIsCollapsed}
106101
/>
107102
)}
108103
{isSaving && <div className="Annotation__actions">Saving...</div>}
109-
{shouldShowActions && (
104+
{showActions && (
110105
<div className="Annotation__actions">
111106
<AnnotationActionBar
112107
annotation={annotation}
@@ -123,6 +118,9 @@ function Annotation({
123118

124119
Annotation.propTypes = {
125120
annotation: propTypes.object.isRequired,
121+
hasAppliedFilter: propTypes.bool.isRequired,
122+
isReply: propTypes.bool,
123+
onToggleReplies: propTypes.func,
126124
replyCount: propTypes.number.isRequired,
127125
showDocumentInfo: propTypes.bool.isRequired,
128126
threadIsCollapsed: propTypes.bool.isRequired,

‎src/sidebar/components/AnnotationMissing.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import classnames from 'classnames';
2+
import { createElement } from 'preact';
3+
import propTypes from 'prop-types';
4+
5+
import AnnotationReplyToggle from './AnnotationReplyToggle';
6+
7+
/** @typedef {import('./Annotation').AnnotationProps} AnnotationProps */
8+
9+
/**
10+
* @typedef {Omit<AnnotationProps, 'annotation'|'showDocumentInfo'|'annotationsService'>} AnnotationMissingProps
11+
*/
12+
13+
/**
14+
* Renders in place of an annotation if a thread's annotation is missing.
15+
*
16+
* @param {AnnotationMissingProps} props
17+
*/
18+
function AnnotationMissing({
19+
hasAppliedFilter,
20+
isReply,
21+
onToggleReplies,
22+
replyCount,
23+
threadIsCollapsed,
24+
}) {
25+
const showReplyToggle = !isReply && !hasAppliedFilter && replyCount > 0;
26+
const isCollapsedReply = isReply && threadIsCollapsed;
27+
28+
return (
29+
<article
30+
className={classnames('Annotation', 'Annotation--missing', {
31+
'is-collapsed': threadIsCollapsed,
32+
})}
33+
>
34+
{!isCollapsedReply && (
35+
<div>
36+
<em>Message not available.</em>
37+
</div>
38+
)}
39+
40+
{!isCollapsedReply && (
41+
<footer className="Annotation__footer">
42+
<div className="Annotation__controls u-layout-row">
43+
{showReplyToggle && (
44+
<AnnotationReplyToggle
45+
onToggleReplies={onToggleReplies}
46+
replyCount={replyCount}
47+
threadIsCollapsed={threadIsCollapsed}
48+
/>
49+
)}
50+
</div>
51+
</footer>
52+
)}
53+
</article>
54+
);
55+
}
56+
57+
AnnotationMissing.propTypes = {
58+
hasAppliedFilter: propTypes.bool,
59+
isReply: propTypes.bool.isRequired,
60+
onToggleReplies: propTypes.func,
61+
replyCount: propTypes.number.isRequired,
62+
threadIsCollapsed: propTypes.bool,
63+
};
64+
65+
export default AnnotationMissing;

‎src/sidebar/components/AnnotationReplyToggle.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { createElement } from 'preact';
2+
import propTypes from 'prop-types';
3+
4+
import Button from './Button';
5+
6+
/**
7+
* @typedef AnnotationReplyToggleProps
8+
* @prop {() => any} onToggleReplies
9+
* @prop {number} replyCount
10+
* @prop {boolean} threadIsCollapsed
11+
*/
12+
13+
/**
14+
* Render a thread-card control to toggle (expand or collapse) all of this
15+
* thread's children.
16+
*
17+
* @param {AnnotationReplyToggleProps} props
18+
*/
19+
function AnnotationReplyToggle({
20+
onToggleReplies,
21+
replyCount,
22+
threadIsCollapsed,
23+
}) {
24+
const toggleAction = threadIsCollapsed ? 'Show replies' : 'Hide replies';
25+
const toggleText = `${toggleAction} (${replyCount})`;
26+
27+
return (
28+
<Button
29+
className="Annotation__reply-toggle"
30+
onClick={onToggleReplies}
31+
buttonText={toggleText}
32+
/>
33+
);
34+
}
35+
36+
AnnotationReplyToggle.propTypes = {
37+
onToggleReplies: propTypes.func,
38+
replyCount: propTypes.number,
39+
threadIsCollapsed: propTypes.bool.isRequired,
40+
};
41+
42+
export default AnnotationReplyToggle;

‎src/sidebar/components/NotebookResultCount.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ function NotebookResultCount() {
2828

2929
const hasResults = rootThread.children.length > 0;
3030

31-
const hasForcedVisible = forcedVisibleCount > 0;
31+
const hasForcedVisible = hasAppliedFilter && forcedVisibleCount > 0;
3232
const matchCount = visibleCount - forcedVisibleCount;
3333
const threadCount = rootThread.children.length;
3434

‎src/sidebar/components/Thread.js‎

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import classnames from 'classnames';
22
import { createElement, Fragment } from 'preact';
3-
import { useMemo } from 'preact/hooks';
3+
import { useCallback, useMemo } from 'preact/hooks';
44

55
import propTypes from 'prop-types';
66
import { useStoreProxy } from '../store/use-store';
77
import { withServices } from '../service-context';
88
import { countHidden, countVisible } from '../helpers/thread';
99

1010
import Annotation from './Annotation';
11+
import AnnotationMissing from './AnnotationMissing';
1112
import Button from './Button';
1213
import ModerationBanner from './ModerationBanner';
1314

@@ -30,6 +31,7 @@ import ModerationBanner from './ModerationBanner';
3031
function Thread({ showDocumentInfo = false, thread, threadsService }) {
3132
// Only render this thread's annotation if it exists and the thread is `visible`
3233
const showAnnotation = thread.annotation && thread.visible;
34+
const showMissingAnnotation = thread.visible && !thread.annotation;
3335

3436
// Render this thread's replies only if the thread is expanded
3537
const showChildren = !thread.collapsed;
@@ -51,32 +53,54 @@ function Thread({ showDocumentInfo = false, thread, threadsService }) {
5153
);
5254

5355
const store = useStoreProxy();
54-
const onToggleReplies = () =>
55-
store.setExpanded(thread.id, !!thread.collapsed);
56+
const hasAppliedFilter = store.hasAppliedFilter();
57+
const onToggleReplies = useCallback(
58+
() => store.setExpanded(thread.id, !!thread.collapsed),
59+
[store, thread.id, thread.collapsed]
60+
);
5661

5762
// Memoize annotation content to avoid re-rendering an annotation when content
5863
// in other annotations/threads change.
59-
const annotationContent = useMemo(
60-
() =>
61-
showAnnotation && (
64+
const annotationContent = useMemo(() => {
65+
if (showAnnotation) {
66+
return (
6267
<Fragment>
6368
<ModerationBanner annotation={thread.annotation} />
6469
<Annotation
6570
annotation={thread.annotation}
71+
hasAppliedFilter={hasAppliedFilter}
72+
isReply={!!thread.parent}
73+
onToggleReplies={onToggleReplies}
6674
replyCount={thread.replyCount}
6775
showDocumentInfo={showDocumentInfo}
6876
threadIsCollapsed={thread.collapsed}
6977
/>
7078
</Fragment>
71-
),
72-
[
73-
showAnnotation,
74-
thread.annotation,
75-
thread.replyCount,
76-
showDocumentInfo,
77-
thread.collapsed,
78-
]
79-
);
79+
);
80+
} else if (showMissingAnnotation) {
81+
return (
82+
<AnnotationMissing
83+
hasAppliedFilter={hasAppliedFilter}
84+
isReply={!!thread.parent}
85+
onToggleReplies={onToggleReplies}
86+
replyCount={thread.replyCount}
87+
threadIsCollapsed={thread.collapsed}
88+
/>
89+
);
90+
} else {
91+
return null;
92+
}
93+
}, [
94+
hasAppliedFilter,
95+
onToggleReplies,
96+
showAnnotation,
97+
showMissingAnnotation,
98+
showDocumentInfo,
99+
thread.annotation,
100+
thread.parent,
101+
thread.replyCount,
102+
thread.collapsed,
103+
]);
80104

81105
return (
82106
<section
@@ -99,12 +123,6 @@ function Thread({ showDocumentInfo = false, thread, threadsService }) {
99123
<div className="Thread__content">
100124
{annotationContent}
101125

102-
{!thread.annotation && (
103-
<div className="Thread__unavailable-message">
104-
<em>Message not available.</em>
105-
</div>
106-
)}
107-
108126
{showHiddenToggle && (
109127
<Button
110128
buttonText={`Show ${countHidden(thread)} more in conversation`}

0 commit comments

Comments
 (0)