Skip to content

Commit e1640f7

Browse files
committed
Add comment mode for content elements
Let reviewers enter a mode where content elements become selectable for commenting. Selecting an element exits the mode, highlights the element with an accent border, opens the comment popover, and hides other badges to reduce clutter. Clicking outside or toggling the toolbar button dismisses the mode. REDMINE-21261
1 parent 0b12c2a commit e1640f7

22 files changed

Lines changed: 490 additions & 80 deletions

entry_types/scrolled/bin/rspec-with-retry-on-timeout

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#!/bin/bash
22

33
# See REDMINE-17430
4-
# Running only the js specs should not take more than 20 seconds.
4+
# Running only the js specs should not take more than 30 seconds.
55
# If Chrome Driver hangs, the timeout command will exit with 137.
66
# Retry or else exit with original exit status of rspec command.
77

88
for i in {1..10}; do
9-
timeout --signal=KILL 30 bin/rspec $@
9+
timeout --signal=KILL 45 bin/rspec $@
1010
e=$?
1111
[[ $e -gt 100 ]] && echo Timeout || exit $e;
1212
done;

entry_types/scrolled/config/locales/de.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1912,7 +1912,9 @@ de:
19121912
content_element_margin_bottom: Unterer Außenabstand
19131913
expose_motif_area: Motiv freilegen
19141914
review:
1915-
toolbar_label: Kommentare
1915+
add_comment: Kommentar hinzufügen
1916+
cancel_add_comment: Abbrechen
1917+
select_content_element: Zum Kommentieren auswählen
19161918
new_topic: Neues Thema
19171919
add_comment_placeholder: Kommentar hinzufügen...
19181920
reply_placeholder: Antworten...

entry_types/scrolled/config/locales/en.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1741,7 +1741,9 @@ en:
17411741
content_element_margin_bottom: Bottom margin
17421742
expose_motif_area: Expose motif
17431743
review:
1744-
toolbar_label: Comments
1744+
add_comment: Add comment
1745+
cancel_add_comment: Cancel
1746+
select_content_element: Select to comment
17451747
new_topic: New topic
17461748
add_comment_placeholder: Add a comment...
17471749
reply_placeholder: Reply...
Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
import React from 'react';
2+
import '@testing-library/jest-dom/extend-expect';
3+
import {act} from '@testing-library/react';
4+
import userEvent from '@testing-library/user-event';
5+
import {useFakeTranslations} from 'pageflow/testHelpers';
6+
7+
import {Entry} from 'frontend/Entry';
8+
import {usePageObjects} from 'support/pageObjects';
9+
import {renderInEntry} from 'support';
10+
import {clearExtensions} from 'frontend/extensions';
11+
import {loadCommentingComponents} from 'frontend/commenting';
12+
13+
describe('add comment mode', () => {
14+
usePageObjects();
15+
16+
useFakeTranslations({
17+
'pageflow_scrolled.review.add_comment': 'Add comment',
18+
'pageflow_scrolled.review.select_content_element': 'Select to comment',
19+
'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...',
20+
'pageflow_scrolled.review.new_topic': 'New topic',
21+
'pageflow_scrolled.review.send': 'Send',
22+
'pageflow_scrolled.review.cancel': 'Cancel',
23+
'pageflow_scrolled.review.cancel_add_comment': 'Cancel add comment'
24+
});
25+
26+
beforeEach(() => {
27+
jest.spyOn(window, 'fetch').mockResolvedValue({
28+
ok: true,
29+
json: () => Promise.resolve({currentUser: null, commentThreads: []})
30+
});
31+
32+
loadCommentingComponents();
33+
});
34+
35+
afterEach(() => {
36+
act(() => clearExtensions());
37+
window.fetch.mockRestore();
38+
});
39+
40+
it('renders add comment button', () => {
41+
const {getByRole} = renderInEntry(<Entry />, {
42+
seed: {
43+
contentElements: [{
44+
typeName: 'withTestId',
45+
configuration: {testId: 5}
46+
}]
47+
}
48+
});
49+
50+
expect(getByRole('button', {name: 'Add comment'})).toBeInTheDocument();
51+
});
52+
53+
it('shows highlight overlays on content elements when activated', async () => {
54+
const user = userEvent.setup();
55+
const {getByRole} = renderInEntry(<Entry />, {
56+
seed: {
57+
contentElements: [{
58+
typeName: 'withTestId',
59+
configuration: {testId: 5}
60+
}]
61+
}
62+
});
63+
64+
await user.click(getByRole('button', {name: 'Add comment'}));
65+
66+
expect(getByRole('button', {name: 'Select to comment'})).toBeInTheDocument();
67+
});
68+
69+
it('opens new thread form when selecting element', async () => {
70+
const user = userEvent.setup();
71+
const {getByRole, getByPlaceholderText} = renderInEntry(<Entry />, {
72+
seed: {
73+
contentElements: [{
74+
typeName: 'withTestId',
75+
configuration: {testId: 5}
76+
}]
77+
}
78+
});
79+
80+
await user.click(getByRole('button', {name: 'Add comment'}));
81+
await user.click(getByRole('button', {name: 'Select to comment'}));
82+
83+
expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument();
84+
});
85+
86+
it('auto expands new thread form when selecting element with existing threads', async () => {
87+
window.fetch.mockResolvedValue({
88+
ok: true,
89+
json: () => Promise.resolve({
90+
currentUser: null,
91+
commentThreads: [{
92+
id: 1,
93+
subjectType: 'ContentElement',
94+
subjectId: 10,
95+
comments: [{id: 1, body: 'Existing comment', createdAt: '2026-01-01T00:00:00Z'}]
96+
}]
97+
})
98+
});
99+
100+
const user = userEvent.setup();
101+
const {getByRole, getByPlaceholderText} = renderInEntry(<Entry />, {
102+
seed: {
103+
contentElements: [{
104+
typeName: 'withTestId',
105+
permaId: 10,
106+
configuration: {testId: 5}
107+
}]
108+
}
109+
});
110+
111+
await user.click(getByRole('button', {name: 'Add comment'}));
112+
await user.click(getByRole('button', {name: 'Select to comment'}));
113+
114+
expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument();
115+
});
116+
117+
it('closes popover when cancelling new thread form on element without threads', async () => {
118+
const user = userEvent.setup();
119+
const {getByRole, queryByRole} = renderInEntry(<Entry />, {
120+
seed: {
121+
contentElements: [{
122+
typeName: 'withTestId',
123+
configuration: {testId: 5}
124+
}]
125+
}
126+
});
127+
128+
await user.click(getByRole('button', {name: 'Add comment'}));
129+
await user.click(getByRole('button', {name: 'Select to comment'}));
130+
await user.click(getByRole('button', {name: 'Cancel'}));
131+
132+
expect(queryByRole('status')).not.toBeInTheDocument();
133+
});
134+
135+
it('exits add comment mode when overlay is clicked', async () => {
136+
const user = userEvent.setup();
137+
const {getByRole, queryByRole} = renderInEntry(<Entry />, {
138+
seed: {
139+
contentElements: [{
140+
typeName: 'withTestId',
141+
configuration: {testId: 5}
142+
}]
143+
}
144+
});
145+
146+
await user.click(getByRole('button', {name: 'Add comment'}));
147+
expect(getByRole('button', {name: 'Select to comment'})).toBeInTheDocument();
148+
149+
await user.click(getByRole('button', {name: 'Select to comment'}));
150+
151+
expect(queryByRole('button', {name: 'Select to comment'})).not.toBeInTheDocument();
152+
});
153+
154+
it('deactivates when clicking toggle button again', async () => {
155+
const user = userEvent.setup();
156+
const {getByRole, queryByRole} = renderInEntry(<Entry />, {
157+
seed: {
158+
contentElements: [{
159+
typeName: 'withTestId',
160+
configuration: {testId: 5}
161+
}]
162+
}
163+
});
164+
165+
await user.click(getByRole('button', {name: 'Add comment'}));
166+
expect(getByRole('button', {name: 'Select to comment'})).toBeInTheDocument();
167+
168+
await user.click(getByRole('button', {name: 'Cancel add comment'}));
169+
170+
expect(queryByRole('button', {name: 'Select to comment'})).not.toBeInTheDocument();
171+
});
172+
173+
it('exits add comment mode when clicking outside', async () => {
174+
const user = userEvent.setup();
175+
const {getByRole, queryByRole} = renderInEntry(<Entry />, {
176+
seed: {
177+
contentElements: [{
178+
typeName: 'withTestId',
179+
configuration: {testId: 5}
180+
}]
181+
}
182+
});
183+
184+
await user.click(getByRole('button', {name: 'Add comment'}));
185+
expect(getByRole('button', {name: 'Select to comment'})).toBeInTheDocument();
186+
187+
await user.click(document.body);
188+
189+
expect(queryByRole('button', {name: 'Select to comment'})).not.toBeInTheDocument();
190+
});
191+
});
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import React, {createContext, useCallback, useContext, useEffect, useMemo, useState} from 'react';
2+
3+
const AddCommentModeContext = createContext({
4+
active: false,
5+
toggle: () => {},
6+
deactivate: () => {}
7+
});
8+
9+
export function AddCommentModeProvider({children}) {
10+
const [active, setActive] = useState(false);
11+
12+
const toggle = useCallback(() => {
13+
setActive(prev => !prev);
14+
}, []);
15+
16+
const deactivate = useCallback(() => {
17+
setActive(false);
18+
}, []);
19+
20+
useEffect(() => {
21+
if (!active) return;
22+
23+
function handlePointerDown(event) {
24+
if (!event.target.closest('[data-add-comment-overlay]') &&
25+
!event.target.closest('[data-add-comment-toggle]')) {
26+
setActive(false);
27+
}
28+
}
29+
30+
document.addEventListener('pointerdown', handlePointerDown);
31+
32+
return () => {
33+
document.removeEventListener('pointerdown', handlePointerDown);
34+
};
35+
}, [active]);
36+
37+
const value = useMemo(() => ({
38+
active,
39+
toggle,
40+
deactivate
41+
}), [active, toggle, deactivate]);
42+
43+
return (
44+
<AddCommentModeContext.Provider value={value}>
45+
{children}
46+
</AddCommentModeContext.Provider>
47+
);
48+
}
49+
50+
export function useAddCommentMode() {
51+
return useContext(AddCommentModeContext);
52+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import React from 'react';
2+
3+
import {useI18n} from '../i18n';
4+
import {useAddCommentMode} from './AddCommentModeProvider';
5+
import {useSelectedSubject} from './SelectedSubjectProvider';
6+
7+
import AddCommentIcon from './images/addComment.svg';
8+
import styles from './AddCommentOverlay.module.css';
9+
10+
export function AddCommentOverlay({permaId}) {
11+
const {t} = useI18n({locale: 'ui'});
12+
const {active, deactivate} = useAddCommentMode();
13+
const {select} = useSelectedSubject('ContentElement', permaId);
14+
15+
if (!active) return null;
16+
17+
function handleClick() {
18+
select({showNewForm: true});
19+
deactivate();
20+
}
21+
22+
return (
23+
<button onClick={handleClick}
24+
data-add-comment-overlay
25+
className={styles.highlight}
26+
aria-label={t('pageflow_scrolled.review.select_content_element')}
27+
title={t('pageflow_scrolled.review.select_content_element')}>
28+
<span className={styles.pill}>
29+
<AddCommentIcon />
30+
</span>
31+
</button>
32+
);
33+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
.highlight {
2+
position: absolute;
3+
inset: 0;
4+
cursor: pointer;
5+
background-color: var(--ui-selection-color-lightest);
6+
border: none;
7+
outline: none;
8+
box-shadow: 0 0 0 2px var(--ui-selection-color),
9+
0 0 0 calc(2px + space(1)) var(--ui-selection-color-light);
10+
display: flex;
11+
align-items: center;
12+
justify-content: center;
13+
}
14+
15+
.pill {
16+
display: flex;
17+
align-items: center;
18+
justify-content: center;
19+
gap: space(1);
20+
width: space(9);
21+
border-radius: rounded(full);
22+
background: var(--ui-accent-color);
23+
color: var(--ui-on-accent-color);
24+
font-family: var(--ui-font-family);
25+
font-size: space(3.5);
26+
font-weight: 600;
27+
padding: space(1.5) space(3);
28+
box-shadow: 0 0 0 space(1) var(--ui-accent-color-glow);
29+
opacity: 0.9;
30+
}
31+
32+
.highlight:hover .pill,
33+
.highlight:focus .pill {
34+
opacity: 1;
35+
}

entry_types/scrolled/package/src/frontend/commenting/ContentElementDecorator.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,22 @@
11
import React from 'react';
2+
import classNames from 'classnames';
23

4+
import {useAddCommentMode} from './AddCommentModeProvider';
5+
import {AddCommentOverlay} from './AddCommentOverlay';
36
import {FloatingAnchor} from './FloatingAnchor';
47
import {Popover} from './Popover';
8+
import {useSelectedSubject} from './SelectedSubjectProvider';
59

610
import styles from './ContentElementDecorator.module.css';
711

812
export function ContentElementDecorator({permaId, children}) {
13+
const {active} = useAddCommentMode();
14+
const {isSelected} = useSelectedSubject('ContentElement', permaId);
15+
916
return (
10-
<div className={styles.wrapper}>
11-
{children}
17+
<div className={classNames(styles.wrapper, {[styles.selected]: isSelected})}>
18+
<div inert={active ? '' : undefined}>{children}</div>
19+
<AddCommentOverlay permaId={permaId} />
1220
<FloatingAnchor>
1321
{({placement}) => (
1422
<Popover subjectType="ContentElement"
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
11
.wrapper {
22
position: relative;
33
}
4+
5+
.selected {
6+
box-shadow: 0 0 0 2px var(--ui-accent-color),
7+
0 0 0 calc(2px + space(1)) var(--ui-accent-color-glow);
8+
}

0 commit comments

Comments
 (0)