Skip to content

Commit efbee3f

Browse files
committed
Remove new thread form cancel button
The new comment form had a cancel button to collapse it back to the thread list. Drop it entirely; users close the thread list popover to undo expanding the form. Removes the onCancel prop from NewThreadForm and its wiring in ThreadList, the popover and the editor new thread view.
1 parent 17b108d commit efbee3f

8 files changed

Lines changed: 9 additions & 57 deletions

File tree

entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ describe('NewThreadView', () => {
1515
useFakeTranslations({
1616
'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...',
1717
'pageflow_scrolled.review.send': 'Send',
18-
'pageflow_scrolled.review.cancel': 'Cancel',
1918
'pageflow_scrolled.editor.new_thread_view.tabs.newComment': 'New topic',
2019
'pageflow_scrolled.editor.new_thread_view.back': 'Comments'
2120
});

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

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,13 @@
11
import React from 'react';
22
import '@testing-library/jest-dom/extend-expect';
33
import userEvent from '@testing-library/user-event';
4-
import {useFakeTranslations} from 'pageflow/testHelpers';
54

65
import {api} from 'frontend/api';
76
import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/commenting';
87

98
describe('add comment mode', () => {
109
useCommentingPageObjects();
1110

12-
useFakeTranslations({
13-
'pageflow_scrolled.review.cancel': 'Cancel'
14-
});
15-
1611
it('renders add comment button', () => {
1712
const entry = renderEntry({
1813
seed: {
@@ -86,24 +81,6 @@ describe('add comment mode', () => {
8681
expect(entry.getNewThreadInput()).toBeInTheDocument();
8782
});
8883

89-
it('closes popover when cancelling new thread form on element without threads', async () => {
90-
const user = userEvent.setup();
91-
const entry = renderEntry({
92-
seed: {
93-
contentElements: [{
94-
typeName: 'withTestId',
95-
configuration: {testId: 5}
96-
}]
97-
}
98-
});
99-
100-
await user.click(entry.getAddCommentButton());
101-
await user.click(entry.getContentElementByTestId(5).getSelectToCommentButton());
102-
await user.click(entry.getByRole('button', {name: 'Cancel'}));
103-
104-
expect(entry.queryAllCommentBadges()).toHaveLength(0);
105-
});
106-
10784
it('exits add comment mode when overlay is clicked', async () => {
10885
const user = userEvent.setup();
10986
const entry = renderEntry({

entry_types/scrolled/package/spec/review/ThreadList-spec.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ describe('ThreadList', () => {
1212
'pageflow_scrolled.review.reply_count.other': '%{count} replies',
1313
'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...',
1414
'pageflow_scrolled.review.new_topic': 'New topic',
15-
'pageflow_scrolled.review.cancel': 'Cancel',
1615
'pageflow_scrolled.review.reply_placeholder': 'Reply...',
1716
'pageflow_scrolled.review.send': 'Send',
1817
'pageflow_scrolled.review.enter_for_new_line': 'Enter for new line',
@@ -694,10 +693,10 @@ describe('ThreadList', () => {
694693
expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument();
695694
});
696695

697-
it('hides form when cancel is clicked', async () => {
696+
it('does not show a cancel button after expanding the new thread form', async () => {
698697
const user = userEvent.setup();
699698

700-
const {getByRole, queryByPlaceholderText} = renderWithReviewState(
699+
const {getByRole, queryByRole, getByPlaceholderText} = renderWithReviewState(
701700
<ThreadList subjectType="ContentElement" subjectId={10} />,
702701
{
703702
commentThreads: [
@@ -709,10 +708,9 @@ describe('ThreadList', () => {
709708
);
710709

711710
await user.click(getByRole('button', {name: 'New topic'}));
712-
await user.click(getByRole('button', {name: 'Cancel'}));
713711

714-
expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument();
715-
expect(getByRole('button', {name: 'New topic'})).toBeInTheDocument();
712+
expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument();
713+
expect(queryByRole('button', {name: 'Cancel'})).not.toBeInTheDocument();
716714
});
717715

718716
it('posts create comment message when replying to thread', async () => {

entry_types/scrolled/package/src/editor/views/NewThreadView.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ const NewThreadFormView = ReviewView.extend({
5454
<NewThreadForm subjectType={subjectType}
5555
subjectId={subjectId}
5656
subjectRange={subjectRange}
57-
onSubmit={leave}
58-
onCancel={leave} />
57+
onSubmit={leave} />
5958
);
6059
}
6160
});

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,7 @@ export function Popover({
8585
subjectId={subjectId}
8686
subjectRange={subjectRange}
8787
showNewForm={showNewForm}
88-
hideNewTopicButton={hideNewTopicButton}
89-
onDismiss={clearSelection} />
88+
hideNewTopicButton={hideNewTopicButton} />
9089
</div>
9190
</FloatingPortal>}
9291
</span>

entry_types/scrolled/package/src/review/NewThreadForm.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {isSubmitShortcut} from './submitShortcut';
88
import SendIcon from './images/send.svg';
99
import styles from './NewThreadForm.module.css';
1010

11-
export function NewThreadForm({subjectType, subjectId, subjectRange, onSubmit, onCancel}) {
11+
export function NewThreadForm({subjectType, subjectId, subjectRange, onSubmit}) {
1212
const {t} = useI18n({locale: 'ui'});
1313
const [body, setBody] = useState('');
1414
const hasText = body.trim().length > 0;
@@ -60,12 +60,6 @@ export function NewThreadForm({subjectType, subjectId, subjectRange, onSubmit, o
6060
<span className={styles.hint}>
6161
{t('pageflow_scrolled.review.enter_for_new_line')}
6262
</span>}
63-
{onCancel &&
64-
<button className={styles.cancelButton}
65-
type="button"
66-
onClick={onCancel}>
67-
{t('pageflow_scrolled.review.cancel')}
68-
</button>}
6963
<button className={styles.submitButton}
7064
type="submit">
7165
<SendIcon /> {t('pageflow_scrolled.review.send')}

entry_types/scrolled/package/src/review/NewThreadForm.module.css

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,6 @@
4242
font-size: space(3);
4343
}
4444

45-
.cancelButton {
46-
font: inherit;
47-
font-weight: 500;
48-
color: var(--ui-on-surface-color-light);
49-
background: none;
50-
border: none;
51-
padding: space(1.5) space(3);
52-
cursor: pointer;
53-
}
54-
5545
.submitButton {
5646
display: flex;
5747
align-items: center;

entry_types/scrolled/package/src/review/ThreadList.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import ChevronIcon from './images/chevron.svg';
1111
import NewTopicIcon from './images/newTopic.svg';
1212
import styles from './ThreadList.module.css';
1313

14-
export function ThreadList({subjectType, subjectId, subjectRange, filter, compareRanges, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed, onDismiss}) {
14+
export function ThreadList({subjectType, subjectId, subjectRange, filter, compareRanges, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed}) {
1515
const {t} = useI18n({locale: 'ui'});
1616
const allActiveThreads = useCommentThreads({subjectType, subjectId, subjectRange}, {resolved: false});
1717
const allResolvedThreads = useCommentThreads({subjectType, subjectId, subjectRange}, {resolved: true});
@@ -57,11 +57,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, compar
5757
<NewThreadForm subjectType={subjectType}
5858
subjectId={subjectId}
5959
subjectRange={subjectRange}
60-
onSubmit={() => setFormToggled(false)}
61-
onCancel={() => {
62-
setFormToggled(false);
63-
if (activeThreads.length === 0 && onDismiss) onDismiss();
64-
}} />}
60+
onSubmit={() => setFormToggled(false)} />}
6561

6662
{noThreads && !showNewForm &&
6763
<p className={styles.blankSlate}>

0 commit comments

Comments
 (0)