Skip to content

Commit 66f7080

Browse files
authored
Merge pull request #2401 from tf/preview-inline-comments
Inline comments in preview mode
2 parents 4599fcf + fe595f8 commit 66f7080

49 files changed

Lines changed: 1188 additions & 58 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

app/assets/stylesheets/pageflow/ui/properties.scss

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@
5858
--ui-box-shadow-2xl: 0 25px 50px -12px rgb(0 0 0 / 0.25);
5959

6060
--ui-accent-color: #ffd24d;
61-
--ui-accent-color-glow: #ffd24d3d;
61+
--ui-accent-color-light: hsla(44, 100%, 65%, 0.6);
62+
--ui-accent-color-lighter: hsla(44, 100%, 65%, 0.3);
63+
--ui-accent-color-lightest: hsla(44, 100%, 65%, 0.1);
6264
--ui-on-accent-color: #795f0f;
6365

6466
--ui-on-button-color: var(--ui-primary-color);

app/controllers/pageflow/review/comment_threads_controller.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ def update
4646
private
4747

4848
def thread_params
49-
params.require(:comment_thread).permit(:subject_type, :subject_id)
49+
permitted = params.require(:comment_thread).permit(:subject_type, :subject_id)
50+
permitted[:subject_range] = params[:comment_thread][:subject_range]&.permit!
51+
permitted
5052
end
5153
end
5254
end

app/models/pageflow/comment_thread.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ module Pageflow
33
class CommentThread < ApplicationRecord
44
include RevisionComponent
55

6+
serialize :subject_range, coder: JSON
7+
68
belongs_to :creator, class_name: 'User'
79
belongs_to :resolver, class_name: 'User', foreign_key: :resolved_by_id, optional: true
810
has_many :comments, dependent: :destroy

app/views/pageflow/review/comment_threads/_comment_thread.json.jbuilder

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ json.call(comment_thread,
55
:perma_id,
66
:subject_type,
77
:subject_id,
8+
:subject_range,
89
:creator_id,
910
:resolved_at,
1011
:created_at,
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
class AddSubjectRangeToCommentThreads < ActiveRecord::Migration[7.1]
2+
def change
3+
add_column :pageflow_comment_threads, :subject_range, :text
4+
end
5+
end

entry_types/scrolled/config/locales/de.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,6 +1915,7 @@ de:
19151915
add_comment: Kommentar hinzufügen
19161916
cancel_add_comment: Abbrechen
19171917
select_content_element: Zum Kommentieren auswählen
1918+
select_text_to_comment: Text zum Kommentieren auswählen
19181919
new_topic: Neues Thema
19191920
add_comment_placeholder: Kommentar hinzufügen...
19201921
reply_placeholder: Antworten...

entry_types/scrolled/config/locales/en.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1744,6 +1744,7 @@ en:
17441744
add_comment: Add comment
17451745
cancel_add_comment: Cancel
17461746
select_content_element: Select to comment
1747+
select_text_to_comment: Select text to comment
17471748
new_topic: New topic
17481749
add_comment_placeholder: Add a comment...
17491750
reply_placeholder: Reply...
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import React from 'react';
2+
import '@testing-library/jest-dom/extend-expect';
3+
import {render} from '@testing-library/react';
4+
import {useFakeTranslations} from 'pageflow/testHelpers';
5+
6+
import {AddCommentHint} from 'frontend/commenting/AddCommentHint';
7+
8+
describe('AddCommentHint', () => {
9+
useFakeTranslations({
10+
'pageflow_scrolled.review.select_text_to_comment': 'Select text to comment'
11+
});
12+
13+
it('renders hint text', () => {
14+
const {getByText} = render(<AddCommentHint />);
15+
16+
expect(getByText('Select text to comment')).toBeInTheDocument();
17+
});
18+
});
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
import React from 'react';
2+
import '@testing-library/jest-dom/extend-expect';
3+
import userEvent from '@testing-library/user-event';
4+
import {useFakeTranslations} from 'pageflow/testHelpers';
5+
6+
import {EditableText} from 'frontend/commenting/EditableText';
7+
import {renderWithCommenting} from 'testHelpers/renderWithCommenting';
8+
import {slateSelection} from 'frontend/commenting/slateSelection';
9+
10+
import {commentHighlightStyles as highlightStyles} from 'pageflow-scrolled/review';
11+
import commentingStyles from 'frontend/commenting/EditableTextHighlight.module.css';
12+
13+
jest.mock('frontend/commenting/slateSelection');
14+
15+
describe('commenting EditableText', () => {
16+
useFakeTranslations({
17+
'pageflow_scrolled.review.select_text_to_comment': 'Select text to comment',
18+
'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...',
19+
'pageflow_scrolled.review.new_topic': 'New topic',
20+
'pageflow_scrolled.review.send': 'Send',
21+
'pageflow_scrolled.review.cancel': 'Cancel'
22+
});
23+
24+
const value = [{type: 'paragraph', children: [{text: 'Some text to comment on'}]}];
25+
26+
it('renders plain text without commenting UI when inlineComments is not set', () => {
27+
const {getByText, queryByText, toggleAddCommentMode} = renderWithCommenting(
28+
<EditableText value={value} />,
29+
{inlineComments: false}
30+
);
31+
32+
toggleAddCommentMode();
33+
34+
expect(getByText('Some text to comment on')).toBeInTheDocument();
35+
expect(queryByText('Select text to comment')).not.toBeInTheDocument();
36+
});
37+
38+
it('shows hint in add comment mode', () => {
39+
const {getByText, toggleAddCommentMode} = renderWithCommenting(
40+
<EditableText value={value} />
41+
);
42+
43+
toggleAddCommentMode();
44+
45+
expect(getByText('Select text to comment')).toBeInTheDocument();
46+
});
47+
48+
it('highlights selected range on mouseup in add comment mode', async () => {
49+
const user = userEvent.setup();
50+
const slateRange = {
51+
anchor: {path: [0, 0], offset: 5},
52+
focus: {path: [0, 0], offset: 9}
53+
};
54+
55+
const {getByText, toggleAddCommentMode} = renderWithCommenting(
56+
<EditableText value={value} />
57+
);
58+
59+
toggleAddCommentMode();
60+
await slateSelection.simulateChange(user, getByText('Some text to comment on'), slateRange);
61+
62+
const highlight = document.querySelector(`.${highlightStyles.highlight}`);
63+
expect(highlight).toBeInTheDocument();
64+
expect(highlight).toHaveTextContent('text');
65+
});
66+
67+
it('hides hint after selection is made', async () => {
68+
const user = userEvent.setup();
69+
const slateRange = {
70+
anchor: {path: [0, 0], offset: 5},
71+
focus: {path: [0, 0], offset: 9}
72+
};
73+
74+
const {getByText, queryByText, toggleAddCommentMode} = renderWithCommenting(
75+
<EditableText value={value} />
76+
);
77+
78+
toggleAddCommentMode();
79+
await slateSelection.simulateChange(user, getByText('Some text to comment on'), slateRange);
80+
81+
expect(queryByText('Select text to comment')).not.toBeInTheDocument();
82+
});
83+
84+
it('shows new thread form after selecting text in add comment mode', async () => {
85+
const user = userEvent.setup();
86+
const slateRange = {
87+
anchor: {path: [0, 0], offset: 5},
88+
focus: {path: [0, 0], offset: 9}
89+
};
90+
91+
const {getByText, getByPlaceholderText, toggleAddCommentMode} = renderWithCommenting(
92+
<EditableText value={value} />
93+
);
94+
95+
toggleAddCommentMode();
96+
await slateSelection.simulateChange(user, getByText('Some text to comment on'), slateRange);
97+
98+
expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument();
99+
});
100+
101+
it('skips hint and highlights immediately when text is already selected', async () => {
102+
const user = userEvent.setup();
103+
const slateRange = {
104+
anchor: {path: [0, 0], offset: 5},
105+
focus: {path: [0, 0], offset: 9}
106+
};
107+
108+
const {getByText, queryByText, toggleAddCommentMode} = renderWithCommenting(
109+
<EditableText value={value} />
110+
);
111+
112+
await slateSelection.simulateChange(user, getByText('Some text to comment on'), slateRange);
113+
toggleAddCommentMode();
114+
115+
expect(queryByText('Select text to comment')).not.toBeInTheDocument();
116+
117+
const highlight = document.querySelector(`.${highlightStyles.highlight}`);
118+
expect(highlight).toBeInTheDocument();
119+
expect(highlight).toHaveTextContent('text');
120+
});
121+
122+
it('highlights thread ranges as clickable', () => {
123+
const subjectRange = {
124+
anchor: {path: [0, 0], offset: 5},
125+
focus: {path: [0, 0], offset: 9}
126+
};
127+
128+
const {container} = renderWithCommenting(
129+
<EditableText value={value} />,
130+
{
131+
commentThreads: [{
132+
id: 1,
133+
subjectType: 'ContentElement',
134+
subjectId: 10,
135+
subjectRange,
136+
comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}]
137+
}]
138+
}
139+
);
140+
141+
const highlight = container.querySelector(`.${highlightStyles.highlight}`);
142+
expect(highlight).toBeInTheDocument();
143+
expect(highlight).toHaveTextContent('text');
144+
expect(highlight).toHaveClass(commentingStyles.clickable);
145+
});
146+
147+
it('shows badge for thread with subjectRange', () => {
148+
const subjectRange = {
149+
anchor: {path: [0, 0], offset: 5},
150+
focus: {path: [0, 0], offset: 9}
151+
};
152+
153+
const {getByRole} = renderWithCommenting(
154+
<EditableText value={value} />,
155+
{
156+
commentThreads: [{
157+
id: 1,
158+
subjectType: 'ContentElement',
159+
subjectId: 10,
160+
subjectRange,
161+
comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}]
162+
}]
163+
}
164+
);
165+
166+
expect(getByRole('status')).toBeInTheDocument();
167+
});
168+
169+
it('does not show new form when selecting range of existing thread', async () => {
170+
const user = userEvent.setup();
171+
const subjectRange = {
172+
anchor: {path: [0, 0], offset: 5},
173+
focus: {path: [0, 0], offset: 9}
174+
};
175+
176+
const {container, queryByPlaceholderText, toggleAddCommentMode} = renderWithCommenting(
177+
<EditableText value={value} />,
178+
{
179+
commentThreads: [{
180+
id: 1,
181+
subjectType: 'ContentElement',
182+
subjectId: 10,
183+
subjectRange,
184+
comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}]
185+
}]
186+
}
187+
);
188+
189+
toggleAddCommentMode();
190+
await slateSelection.simulateChange(
191+
user,
192+
container.querySelector('[data-slate-editor]'),
193+
subjectRange
194+
);
195+
196+
expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument();
197+
});
198+
});

entry_types/scrolled/package/spec/frontend/features/addCommentMode-spec.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import userEvent from '@testing-library/user-event';
55
import {useFakeTranslations} from 'pageflow/testHelpers';
66

77
import {Entry} from 'frontend/Entry';
8+
import {api} from 'frontend/api';
89
import {usePageObjects} from 'support/pageObjects';
910
import {renderInEntry} from 'support';
1011
import {clearExtensions} from 'frontend/extensions';
@@ -170,6 +171,29 @@ describe('add comment mode', () => {
170171
expect(queryByRole('button', {name: 'Select to comment'})).not.toBeInTheDocument();
171172
});
172173

174+
it('does not show overlay on inlineComments elements', async () => {
175+
api.contentElementTypes.register('withInlineComments', {
176+
component: function WithInlineComments() {
177+
return <div data-testid="inlineCommentsElement" />;
178+
},
179+
inlineComments: true
180+
});
181+
182+
const user = userEvent.setup();
183+
const {getByRole, queryAllByRole} = renderInEntry(<Entry />, {
184+
seed: {
185+
contentElements: [
186+
{typeName: 'withInlineComments'},
187+
{typeName: 'withTestId', configuration: {testId: 5}}
188+
]
189+
}
190+
});
191+
192+
await user.click(getByRole('button', {name: 'Add comment'}));
193+
194+
expect(queryAllByRole('button', {name: 'Select to comment'})).toHaveLength(1);
195+
});
196+
173197
it('exits add comment mode when clicking outside', async () => {
174198
const user = userEvent.setup();
175199
const {getByRole, queryByRole} = renderInEntry(<Entry />, {

0 commit comments

Comments
 (0)