[web] Remove duplicate `thread[Name/Description]Disabled` props in `ThreadSettingsGeneralTab` component
ClosedPublic
Actions

Authored by atul on Apr 24 2022, 1:05 PM.

Details

Reviewers

• jacek
tomek
ashoat

Commits

rCOMM024a3be9ffb0: [web] Remove duplicate `thread[Name/Description]Disabled` props in…

Summary

These props both get inputDisabled passed in from the parent component, so they can be "flattened" into a single prop.

Depends on D3825

Test Plan

Open ThreadSettingsModal
Make sure that the "General" tab continues to look/work as expected

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D3826 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Apr 24 2022, 1:05 PM

Herald added subscribers: • abosh, • karol, • adrian, ashoat. · View Herald TranscriptApr 24 2022, 1:05 PM

Harbormaster completed remote builds in B8486: Diff 11831.Apr 24 2022, 1:10 PM

atul requested review of this revision.Apr 24 2022, 1:10 PM

atul added a child revision: D3827: [web] Push `onChange[Name/Description/Color]` callbacks down to `ThreadSettingsGeneralTab` component.Apr 24 2022, 1:49 PM

Just wondering: are we planning to have threads with one input enabled and another disabled? It feels like changing a description is less destructive than changing a name and more users could be allowed to do that. On the other hand, reintroducing this option shouldn't be hard. Adding @ashoat to get his opinion.

rebase

In D3826#106968, @palys-swm wrote:

Just wondering: are we planning to have threads with one input enabled and another disabled? It feels like changing a description is less destructive than changing a name and more users could be allowed to do that. On the other hand, reintroducing this option shouldn't be hard. Adding @ashoat to get his opinion.

That's a good point, I could definitely see us having more fine-grained thread setting permissions in the future.

For now just maintaining the existing behavior, which I think should be fine for a refactoring stack of diffs.

Harbormaster completed remote builds in B8567: Diff 11946.Apr 26 2022, 10:32 AM

We can refactor later if we need to

This revision is now accepted and ready to land.Apr 27 2022, 11:43 AM

rebase before landing

Harbormaster completed remote builds in B8669: Diff 12070.Apr 28 2022, 1:27 PM

rebase before landing, actually

This revision was landed with ongoing or failed builds.Apr 28 2022, 1:33 PM

Closed by commit rCOMM024a3be9ffb0: [web] Remove duplicate `thread[Name/Description]Disabled` props in… (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM024a3be9ffb0: [web] Remove duplicate `thread[Name/Description]Disabled` props in….

Harbormaster failed remote builds in B8672: Diff 12073!Apr 28 2022, 1:36 PM

Revision Contents
Changeset List

Path

Size

web/

modals/

threads/

thread-settings-general-tab.react.js

10 lines

thread-settings-modal.react.js

3 lines

Diff 12073

View Options

web/modals/threads/thread-settings-general-tab.react.js

Show All 36 Lines	const {
threadInfo,		threadInfo,
setErrorMessage,		setErrorMessage,
} = props;		} = props;

const modalContext = useModalContext();		const modalContext = useModalContext();
const dispatchActionPromise = useDispatchActionPromise();		const dispatchActionPromise = useDispatchActionPromise();
const callDeleteThread = useServerCall(deleteThread);		const callDeleteThread = useServerCall(deleteThread);

		const accountPasswordInputRef = React.useRef();

const deleteThreadAction = React.useCallback(async () => {		const deleteThreadAction = React.useCallback(async () => {
try {		try {
const response = await callDeleteThread(threadInfo.id, accountPassword);		const response = await callDeleteThread(threadInfo.id, accountPassword);
modalContext.clearModal();		modalContext.clearModal();
return response;		return response;
} catch (e) {		} catch (e) {
setErrorMessage(		setErrorMessage(
e.message === 'invalid_credentials' ? 'wrong password' : 'unkown error',		e.message === 'invalid_credentials' ? 'wrong password' : 'unkown error',
);		);
setAccountPassword('');		setAccountPassword('');
// TODO: accountPasswordInput.focus()		accountPasswordInputRef.current?.focus();
// (once ref is moved up to functional component)
throw e;		throw e;
}		}
}, [		}, [
accountPassword,		accountPassword,
callDeleteThread,		callDeleteThread,
modalContext,		modalContext,
setAccountPassword,		setAccountPassword,
setErrorMessage,		setErrorMessage,
Show All 23 Lines	<>
<div className={css.form_title}>Account password</div>		<div className={css.form_title}>Account password</div>
<div className={css.form_content}>		<div className={css.form_content}>
<input		<input
type="password"		type="password"
placeholder="Personal account password"		placeholder="Personal account password"
value={accountPassword}		value={accountPassword}
onChange={onChangeAccountPassword}		onChange={onChangeAccountPassword}
disabled={inputDisabled}		disabled={inputDisabled}
		ref={accountPasswordInputRef}
/>		/>
</div>		</div>
</div>		</div>
<Button onClick={onDelete} variant="danger" disabled={inputDisabled}>		<Button onClick={onDelete} variant="danger" disabled={inputDisabled}>
Delete		Delete
</Button>		</Button>
</>		</>
);		);
}		}

export default ThreadSettingsDeleteTab;		export default ThreadSettingsDeleteTab;