-
Notifications
You must be signed in to change notification settings - Fork 235
fix: Disable logo update button when no file is selected #10587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev/7.4.x
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
| const [isImageCropModalShow, setIsImageCropModalShow] = useState<boolean>(false); | ||
| const [isDefaultLogoSelected, setIsDefaultLogoSelected] = useState<boolean>(isDefaultLogo ?? true); | ||
| const [retrieveError, setRetrieveError] = useState<any>(); | ||
| const [isFileSelected, setIsFileSelected] = useState<boolean>(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ファイルが選択されているか否かは uploadLogoSrc を見れば良さそう
目的モーダル内でキャンセルボタンを押した場合に、 やりたい動作モーダルでキャンセルボタンを押した後には、ファイル名を消す動作が正しい このように書いた理由inputタグはjavascript側で任意に選択したファイル名を消す機能がなく、もし消したいなら新しいinputタグを再生成させなければなりません。なので、inputタグにkeyを設定し、 参考文献:
|
| <input type="file" onChange={onSelectFile} name="brandLogo" accept="image/*" /> | ||
| <input | ||
| type="file" | ||
| key={fileInputKey} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この部分でinputタグのkeyに呼び出した現在のミリ秒の文字列を代入しています。この処理により毎回resetFileInput()が呼び出されればinputタグが再構築されることによって空のinputタグが生成され、モーダルのキャンセルボタンを押したとしてもすでに選んでしまったfile名が残らないようにしました。
| onModalClose={() => setIsImageCropModalShow(false)} | ||
| onModalClose={() => { | ||
| if (!isImageCropped) { | ||
| resetFileInput(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jsx の部分にロジックがあると見にくいので closeImageCropModalHandler みたいなメソッドを作ってそれを渡すようにしてください
| }, [t, isDefaultLogoSelected]); | ||
|
|
||
| const resetFileInput = useCallback(() => { | ||
| setFileInputKey(Date.now().toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここで呼び出した瞬間のミリ秒時間を引数として入力することによって呼び出したら必ず異なるkeyがinput タグに入力されることになります。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key をリセットするやり方がちょっとトリッキーなので useRef 使って実装してみてください。
例
const fileInputRef = useRef<HTMLInputElement | null>(null);
const clearFileInput = useCallback(() => {
if (fileInputRef.current) {
fileInputRef.current.value = '';
}
}, []);| setUploadLogoSrc(null); | ||
| setIsImageCropped(false); | ||
| setIsImageCropModalShow(false); | ||
| resetFileInput(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }, [t, isDefaultLogoSelected]); | ||
|
|
||
| const resetFileInput = useCallback(() => { | ||
| setFileInputKey(Date.now().toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key をリセットするやり方がちょっとトリッキーなので useRef 使って実装してみてください。
例
const fileInputRef = useRef<HTMLInputElement | null>(null);
const clearFileInput = useCallback(() => {
if (fileInputRef.current) {
fileInputRef.current.value = '';
}
}, []);| onModalClose={() => setIsImageCropModalShow(false)} | ||
| onModalClose={() => { | ||
| if (!isImageCropped) { | ||
| resetFileInput(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jsx の部分にロジックがあると見にくいので closeImageCropModalHandler みたいなメソッドを作ってそれを渡すようにしてください
| <AdminUpdateButtonRow | ||
| onClick={onClickSubmit} | ||
| disabled={retrieveError != null | ||
| || (!isDefaultLogoSelected && uploadLogoSrc == null && !isCustomizedLogoUploaded)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isUpdateButtonDisabled みたいな変数を作ってそれを渡すようにしてください
| src={uploadLogoSrc} | ||
| onModalClose={() => setIsImageCropModalShow(false)} | ||
| onModalClose={() => { | ||
| if (!isImageCropped) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onImageProcessCompleted というメソッドが ImageCropModal 側に提供されているみたいですが、ためしてみました?
state が増えると実装が複雑になるので増やすのは慎重にしたいです


TASK
https://redmine.weseek.co.jp/issues/175223
https://redmine.weseek.co.jp/issues/176103
行ったこと
ファイル選択がされていない状態で更新ボタンを押そうとしたときにはボタンがdisabledになるようにしました。
ファイル選択時にファイル名が残ってしまうバグを直しました。
具体例