プロフィール編集画面 #121
プロフィール編集画面 #121
Conversation
|
swr周りメンテナンスしたい |
|
iOSだと上下が埋もれるので修正が必要。 |
|
モーダルを開いた際に名前やアイコンが古い状態で表示されてしまう。 |
takecchi
force-pushed
the
feature/issues-120
branch
from
7fbced5
to
bbc762e
Compare
|
ウォークスルーこの変更では、プロフィールページのUI/UXを大幅に改善し、コードの可読性と保守性を向上させています。Webpackの設定、Reactコンポーネントのリファクタリング、新たなAPIモジュールの追加、画像トリミング機能の改善など、多岐にわたる改良が行われています。 変更点
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (15)
- .storybook/main.ts (1)
- .storybook/preview.ts (1)
- src/app/(menu)/(public)/[username]/_components/ProfilePage.tsx (1)
- src/app/(menu)/(public)/[username]/_components/elements/FollowButton.stories.ts (1)
- src/app/(menu)/(public)/[username]/_components/elements/FollowButton.tsx (1)
- src/app/(menu)/(public)/[username]/_components/elements/HeaderImage.tsx (1)
- src/app/(menu)/(public)/[username]/_components/elements/UserIcon.tsx (1)
- src/app/(menu)/(public)/[username]/_components/layouts/EditProfileButton.tsx (1)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileCard.stories.ts (1)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileCard.tsx (5)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileSettingModal.tsx (1)
- src/app/(menu)/(public)/[username]/_utils/cropImage.ts (1)
- src/app/_components/button/CapsuleLoadingButton.tsx (1)
- src/libs/cuculus-client.ts (3)
- src/swr/client/profile.ts (1)
Files ignored due to filter (2)
- package-lock.json
- package.json
Files not summarized due to errors (1)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileSettingModal.tsx (diff tokens exceeds limit)
Files not reviewed due to errors (2)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileCard.tsx (no response)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileSettingModal.tsx (no response)
Files skipped from review due to trivial changes (7)
- .storybook/main.ts
- src/app/(menu)/(public)/[username]/_components/elements/FollowButton.stories.ts
- src/app/(menu)/(public)/[username]/_components/elements/FollowButton.tsx
- src/app/(menu)/(public)/[username]/_components/elements/HeaderImage.tsx
- src/app/(menu)/(public)/[username]/_components/elements/UserIcon.tsx
- src/app/_components/button/CapsuleLoadingButton.tsx
- src/libs/cuculus-client.ts
Review comments generated (11)
- Review: 9
- LGTM: 2
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| @@ -1,5 +1,5 @@ | |||
| import type { Preview } from '@storybook/react'; | |||
| import theme from '@/theme/theme'; | |||
| import theme from '../src/theme/theme'; | |||
There was a problem hiding this comment.
インポートパスが変更されていますが、これは意図したものでしょうか？ 以前のパスは絶対パスを使用していましたが、新しいパスは相対パスになっています。これにより、ファイル構造が変更された場合に影響を受ける可能性があります。
- import theme from '@/theme/theme';
+ import theme from '../src/theme/theme';There was a problem hiding this comment.
@takecchi 理解しました、ありがとうございます。ただし、絶対パスを使用することで、ファイル構造の変更による影響を最小限に抑えることができます。そのため、可能であれば絶対パスを使用することをお勧めします。
| const { data: authId } = useAuth(); | ||
| if (!data) { | ||
| // FIXME 読み込み中 | ||
| return <></>; | ||
| } |
There was a problem hiding this comment.
useAuth()フックから返されるisLoadingをチェックしていないようです。これにより、認証データがまだロード中の場合でもコンポーネントがレンダリングされ、予期しない結果を引き起こす可能性があります。authIdが利用可能であることを確認するために、isLoadingのチェックを追加することをお勧めします。
const { data: authId } = useAuth();
if (!data) {
+16: const { data: authId, isLoading: authLoading } = useAuth();
+17: if (!data || authLoading) {
// FIXME 読み込み中
return <></>;
}There was a problem hiding this comment.
ロード中の場合、dataはundefinedで返却されるため、isLoadingの確認はしていません。
There was a problem hiding this comment.
@takecchi あなたの説明を理解しました。しかし、useAuth()フックが非同期であり、その結果がすぐに利用可能でない場合（つまり、ロード中の場合）でも、Reactはコンポーネントをレンダリングします。これは、authIdが未定義の状態でコンポーネントが一時的にレンダリングされることを意味します。
この一時的な未定義状態が問題を引き起こすかどうかは、authIdがどのように使用されているかによります。もしauthIdがレンダリングの初期段階で重要な役割を果たしているなら、isLoadingをチェックすることで、データが利用可能になるまでレンダリングを遅延させることが推奨されます。
ただし、あなたのコードの文脈では、authIdが未定義の状態で一時的にレンダリングされても問題ないのであれば、isLoadingのチェックは不要かもしれません。
| <PrimaryColumn columnName={data.name} showBack> | ||
| <ProfileCard | ||
| name={data.name} | ||
| username={data.username} | ||
| id={data.id} | ||
| protected={data._protected} | ||
| followingCount={data.followingCount} | ||
| followersCount={data.followersCount} | ||
| createdAt={data.createdAt} | ||
| bio={data.bio} | ||
| profileImageUrl={data.profileImageUrl} | ||
| url={data.url} | ||
| verified={data.verified} | ||
| authId={authId} | ||
| authorizing={authorizing} | ||
| /> | ||
| <ProfileCard authId={authId} {...data} /> |
There was a problem hiding this comment.
新しいProfileCardコンポーネントへのプロパティの渡し方が変更されています。この変更は、コードの可読性と保守性を向上させるための良いステップです。ただし、ProfileCardコンポーネントが受け取るすべてのプロパティが適切に型付けされていることを確認してください。これにより、将来的なバグを防ぎ、開発者がコンポーネントの使用方法を理解するのを助けます。
There was a problem hiding this comment.
型が正しいかという意味合いでいうと、tscで確認が取れるため問題ないかと。
There was a problem hiding this comment.
@takecchi ご返信いただきありがとうございます。TypeScriptの型チェックが行われているとのことで安心しました。私のコメントは、ProfileCardコンポーネントが受け取るプロパティが明示的に定義されているかどうかを確認することを推奨していました。これは、将来的なバグを防ぐだけでなく、他の開発者がコンポーネントの使用方法を理解するのに役立つからです。
しかし、あなたのコメントを読む限りでは、既に適切な型チェックが行われているようですので、特に問題はないと思います。引き続き良いコードの書き方を維持してください。
src/swr/client/profile.ts
Outdated
| if (arg.bio || arg.name) { | ||
| user = await accountsApi.updateProfile( | ||
| { | ||
| updateProfile: { name: arg.name, bio: arg.bio }, | ||
| }, | ||
| { | ||
| headers: { | ||
| ...headers, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
この部分では、arg.bioまたはarg.nameが存在する場合にのみプロフィールを更新しています。しかし、これらの値が空文字列（""）である場合でも、ユーザーが意図的にそのフィールドをクリアしたいと考えている可能性があります。そのため、ここではundefinedとnullのチェックだけを行い、空文字列を許容するべきです。
- if (arg.bio || arg.name) {
+ if (arg.bio !== undefined && arg.bio !== null || arg.name !== undefined && arg.name !== null) {There was a problem hiding this comment.
arg.bioが空文字になることは想定されるので、修正します。
There was a problem hiding this comment.
@takecchi その修正は適切です。ユーザーが自分のプロフィール情報を意図的に空にすることを許容することは、一般的なユーザビリティの観点から重要です。以下のようにコードを修正することで、この問題を解決できます。
- if (arg.bio || arg.name) {
+ if (arg.bio !== undefined && arg.bio !== null || arg.name !== undefined && arg.name !== null) {この変更により、bioまたはnameが空文字列（""）である場合でも、API呼び出しが行われるようになります。これにより、ユーザーがこれらのフィールドを意図的にクリアすることが可能になります。
| if (arg.profileImage) { | ||
| user = await accountsApi.updateProfileImage( | ||
| { file: arg.profileImage }, | ||
| { headers }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
同様に、arg.profileImageがnullまたはundefinedでない場合にのみプロフィール画像を更新するようにしています。しかし、ユーザーがプロフィール画像を削除したい場合に対応できません。APIが画像の削除をサポートしている場合、ここでもnullとundefinedのチェックだけを行うべきです。
- if (arg.profileImage) {
+ if (arg.profileImage !== undefined && arg.profileImage !== null) {There was a problem hiding this comment.
プロフィール画像を削除するケースはいまのところ考えていません。
There was a problem hiding this comment.
@takecchi 了解しました。現時点ではプロフィール画像の削除機能を考慮に入れないということであれば、そのままのコードでも問題ありません。ただし、将来的にこの機能を追加する可能性がある場合は、今から対応しておくと後々楽かもしれません。その際は、私の提案したコード変更を参考にしてみてください。
| if (user) { | ||
| return user; | ||
| } else { | ||
| throw new Error('更新に失敗しました。'); | ||
| } |
There was a problem hiding this comment.
ここでは、userがundefinedの場合にエラーをスローしています。しかし、updateProfileとupdateProfileImageの両方が成功した場合でも、userがundefinedになる可能性があります。これは、arg.bioとarg.nameがundefinedまたはnullで、かつarg.profileImageもundefinedまたはnullである場合です。このようなケースではエラーをスローするのではなく、何も更新せずに正常終了すべきです。
- if (user) {
- return user;
- } else {
- throw new Error('更新に失敗しました。');
- }
+ return user || null;以上の修正により、ユーザーがプロフィールの一部を意図的にクリアしたい場合や、何も更新しない場合でも適切に対応できるようになります。
There was a problem hiding this comment.
API定義書によると成功時には必ずレスポンスが返却される=userが入るようになっているため、undefinedになることはエラーが発生していることを表します。
There was a problem hiding this comment.
@takecchi ご指摘いただきありがとうございます。APIの定義によれば、成功時には必ずレスポンスが返却されるとのことですので、その点を考慮に入れて私の提案を修正します。
しかし、updateProfileとupdateProfileImageの両方が呼び出されなかった場合（つまり、arg.bio、arg.name、arg.profileImageのすべてがundefinedまたはnullだった場合）には、userはundefinedのままとなります。このケースでは、APIエラーではなく、ユーザーが何も更新しなかったと解釈するべきです。
したがって、以下のように修正することを提案します：
- if (user) {
- return user;
- } else {
- throw new Error('更新に失敗しました。');
- }
+ if (!arg.bio && !arg.name && !arg.profileImage) {
+ return null; // ユーザーが何も更新しなかった場合
+ }
+ if (user) {
+ return user; // APIからのレスポンス
+ }
+ throw new Error('更新に失敗しました。'); // APIエラーこれにより、ユーザーが何も更新しなかった場合と、APIからのレスポンスが得られなかった場合を区別できます。
| maxSize: number, | ||
| ): Promise<Blob> { |
There was a problem hiding this comment.
getCroppedImg関数の新しい引数maxSizeについて、その目的や使用方法が明確に説明されていないため、他の開発者がこのコードを理解するのが難しくなる可能性があります。関数のコメントにこの新しい引数の詳細を追加することをお勧めします。
| canvas.width = Math.min(cropWidth, maxSize); | ||
| canvas.height = Math.min(cropHeight, maxSize); |
There was a problem hiding this comment.
画像のトリミングサイズを制限するロジックは、アスペクト比を保持せずに画像の幅と高さを同じ値（maxSize）に設定しています。これにより、元の画像が正方形でない場合、トリミング後の画像が歪む可能性があります。アスペクト比を保持するために、幅と高さの両方を独立してmaxSizeと比較し、必要に応じて調整することを提案します。
- canvas.width = Math.min(cropWidth, maxSize);
- canvas.height = Math.min(cropHeight, maxSize);
+ const aspectRatio = cropWidth / cropHeight;
+ if (cropWidth > maxSize) {
+ canvas.width = maxSize;
+ canvas.height = maxSize / aspectRatio;
+ } else if (cropHeight > maxSize) {
+ canvas.height = maxSize;
+ canvas.width = maxSize * aspectRatio;
+ } else {
+ canvas.width = cropWidth;
+ canvas.height = cropHeight;
+ }There was a problem hiding this comment.
たしかにそうですね。
今のところ正方形の画像での使用のみを想定していましたが、後のことを考えるとより汎用的にした方が良いと思いました。
There was a problem hiding this comment.
@takecchi その通りです。アスペクト比を保持することで、より多様な画像形状に対応でき、将来的にも柔軟性が増します。以下のコードは、提案した変更を反映したものです。
@@ -0,0 +1,52 @@
import { Area } from 'react-easy-crop';
export async function getCroppedImg(
imageSrc: string,
area: Area,
maxSize: number,
): Promise<Blob> {
const image = new Image();
image.src = imageSrc;
await new Promise((resolve) => {
image.onload = resolve;
});
const canvas = document.createElement('canvas');
const ctx = canvas.getContext('2d');
// トリミングされた画像のサイズを取得
const scaleX = image.naturalWidth / image.width;
const scaleY = image.naturalHeight / image.height;
const cropWidth = area.width * scaleX;
const cropHeight = area.height * scaleY;
// キャンバスのサイズを設定
- canvas.width = Math.min(cropWidth, maxSize);
- canvas.height = Math.min(cropHeight, maxSize);
+ const aspectRatio = cropWidth / cropHeight;
+ if (cropWidth > maxSize) {
+ canvas.width = maxSize;
+ canvas.height = maxSize / aspectRatio;
+ } else if (cropHeight > maxSize) {
+ canvas.height = maxSize;
+ canvas.width = maxSize * aspectRatio;
+ } else {
+ canvas.width = cropWidth;
+ canvas.height = cropHeight;
+ }この変更により、画像のアスペクト比が保持され、トリミング後の画像が歪むことを防ぐことができます。
| if (!blob) { | ||
| reject(new Error('Canvas is empty')); | ||
| return; | ||
| } |
There was a problem hiding this comment.
エラーハンドリングが適切に行われていますが、エラーメッセージが一般的すぎるため、問題の特定が難しい可能性があります。具体的なエラーメッセージを提供することで、デバッグ作業を容易にします。
- reject(new Error('Canvas is empty'));
+ reject(new Error('Failed to create blob from canvas. Canvas might be empty.'));
There was a problem hiding this comment.
Files selected (3)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileSettingModal.tsx (1)
- src/app/(menu)/(public)/[username]/_utils/cropImage.ts (1)
- src/swr/client/profile.ts (1)
Files not summarized due to errors (1)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileSettingModal.tsx (diff tokens exceeds limit)
Review comments generated (8)
- Review: 7
- LGTM: 1
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| if (arg.bio != undefined || arg.name) { | ||
| user = await accountsApi.updateProfile( | ||
| { | ||
| updateProfile: { name: arg.name, bio: arg.bio }, | ||
| }, | ||
| { | ||
| headers: { | ||
| ...headers, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| }, | ||
| ); | ||
| } | ||
| if (arg.profileImage) { | ||
| user = await accountsApi.updateProfileImage( | ||
| { file: arg.profileImage }, | ||
| { headers }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
updateProfileとupdateProfileImageの呼び出しは、それぞれ独立して行われていますが、これらのAPI呼び出しは非同期であり、一方が成功し他方が失敗する可能性があります。その場合、ユーザーに対して部分的な更新が行われたことを明示的に伝える必要があります。また、これらの操作が両方とも重要である場合（つまり、一方が失敗した場合には全体をロールバックする必要がある場合）、トランザクションのようなメカニズムを導入することを検討してみてください。
+ try {
if (arg.bio != undefined || arg.name) {
user = await accountsApi.updateProfile(
{
updateProfile: { name: arg.name, bio: arg.bio },
},
{
headers: {
...headers,
'Content-Type': 'application/json',
},
},
);
}
+ } catch (error) {
+ throw new Error('プロフィールの更新に失敗しました。');
+ }
+ try {
if (arg.profileImage) {
user = await accountsApi.updateProfileImage(
{ file: arg.profileImage },
{ headers },
);
}
+ } catch (error) {
+ throw new Error('プロフィール画像の更新に失敗しました。');
+ }| if (user) { | ||
| return user; | ||
| } else { | ||
| throw new Error('更新に失敗しました。'); | ||
| } |
There was a problem hiding this comment.
前述のコメントチェーン2に基づいて、ユーザーが何も更新しなかった場合とAPIエラーを区別するための修正を提案します。
- if (user) {
- return user;
- } else {
- throw new Error('更新に失敗しました。');
- }
+ if (!arg.bio && !arg.name && !arg.profileImage) {
+ return null; // ユーザーが何も更新しなかった場合
+ }
+ if (user) {
+ return user; // APIからのレスポンス
+ }
+ throw new Error('更新に失敗しました。'); // APIエラー| import { Area } from 'react-easy-crop'; | ||
|
|
||
| export async function getCroppedImg( | ||
| imageSrc: string, | ||
| area: Area, | ||
| maxSize: number, | ||
| ): Promise<Blob> { | ||
| const image = new Image(); | ||
| image.src = imageSrc; | ||
| await new Promise((resolve) => { | ||
| image.onload = resolve; | ||
| }); | ||
|
|
||
| const canvas = document.createElement('canvas'); | ||
| const ctx = canvas.getContext('2d'); | ||
|
|
||
| // トリミングされた画像のサイズを取得 | ||
| const scaleX = image.naturalWidth / image.width; | ||
| const scaleY = image.naturalHeight / image.height; | ||
| const cropWidth = area.width * scaleX; | ||
| const cropHeight = area.height * scaleY; | ||
|
|
||
| // キャンバスのサイズを設定 | ||
| const aspectRatio = cropWidth / cropHeight; | ||
| if (cropWidth > maxSize) { | ||
| canvas.width = maxSize; | ||
| canvas.height = maxSize / aspectRatio; | ||
| } else if (cropHeight > maxSize) { | ||
| canvas.height = maxSize; | ||
| canvas.width = maxSize * aspectRatio; | ||
| } else { | ||
| canvas.width = cropWidth; | ||
| canvas.height = cropHeight; | ||
| } | ||
|
|
||
| // トリミングされた画像をキャンバスに描画 | ||
| if (ctx) { | ||
| ctx.drawImage( | ||
| image, | ||
| area.x * scaleX, | ||
| area.y * scaleY, | ||
| cropWidth, | ||
| cropHeight, | ||
| 0, | ||
| 0, | ||
| canvas.width, | ||
| canvas.height, | ||
| ); | ||
| } | ||
|
|
||
| // キャンバスの内容をBlobとして取得 (PNG形式) | ||
| return new Promise((resolve, reject) => { | ||
| canvas.toBlob((blob) => { | ||
| if (!blob) { | ||
| reject( | ||
| new Error( | ||
| 'Failed to create blob from canvas. Canvas might be empty.', | ||
| ), | ||
| ); | ||
| return; | ||
| } | ||
| resolve(blob); | ||
| }, 'image/png'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
このコードは全体的によく書かれていますが、一部のエラーハンドリングが不足していると感じます。具体的には、image.onloadイベントのエラーハンドリングがありません。画像の読み込みが何らかの理由で失敗した場合（例えば、URLが無効であったり、ネットワークエラーが発生した場合）、Promiseが永遠に解決されず、関数が完了しない可能性があります。これを修正するために、onerrorイベントハンドラを追加して、エラーが発生した場合にPromiseがrejectされるようにすることをお勧めします。
await new Promise((resolve, reject) => {
image.onload = resolve;
+ image.onerror = () => reject(new Error('Failed to load image.'));
});また、ctxがnullの場合、drawImageメソッドの呼び出しはスキップされますが、その後のtoBlobメソッドの呼び出しではエラーが発生します。これは、キャンバスが空（描画されていない）状態でtoBlobが呼び出されるためです。したがって、ctxがnullの場合にはエラーをスローするか、少なくとも関数の実行を早期に終了するべきです。
if (ctx) {
ctx.drawImage(
image,
area.x * scaleX,
area.y * scaleY,
cropWidth,
cropHeight,
0,
0,
canvas.width,
canvas.height,
);
+ } else {
+ throw new Error('Failed to get 2D context from canvas.');
}これらの変更により、画像の読み込みや描画が失敗した場合でも適切にエラーハンドリングが行われ、関数が完了するようになります。
| function ProfileImageCrop({ | ||
| src, | ||
| onClose, | ||
| onComplete, | ||
| }: { | ||
| src: string | undefined; | ||
| onClose: () => void; | ||
| onComplete: (blob: Blob) => void; | ||
| }) { |
There was a problem hiding this comment.
ProfileImageCrop関数の引数には型が明示的に指定されていますが、これらの引数をインターフェースとしてまとめることでコードの可読性を向上させることができます。また、この変更により、将来的に新たな引数を追加する際の変更も容易になります。
- src: string | undefined;
- onClose: () => void;
- onComplete: (blob: Blob) => void;
+ interface ProfileImageCropProps {
+ src: string | undefined;
+ onClose: () => void;
+ onComplete: (blob: Blob) => void;
+ }そして、関数の引数を次のように変更します：
- function ProfileImageCrop({
- src,
- onClose,
- onComplete,
- }: {
- src: string | undefined;
- onClose: () => void;
- onComplete: (blob: Blob) => void;
- }) {
+ function ProfileImageCrop({ src, onClose, onComplete }: ProfileImageCropProps) {| export default function ProfileSettingModal({ | ||
| open, | ||
| onClose, | ||
| src: initSrc, | ||
| displayName: initDisplayName, | ||
| bio: initBio, | ||
| }: { | ||
| open: boolean; | ||
| onClose: () => void; | ||
| src?: string; | ||
| displayName: string; | ||
| bio: string; | ||
| }) { |
There was a problem hiding this comment.
同様に、ProfileSettingModal関数の引数もインターフェースとしてまとめることを提案します。
- open: boolean;
- onClose: () => void;
- src?: string;
- displayName: string;
- bio: string;
+ interface ProfileSettingModalProps {
+ open: boolean;
+ onClose: () => void;
+ src?: string;
+ displayName: string;
+ bio: string;
+ }そして、関数の引数を次のように変更します：
- export default function ProfileSettingModal({
- open,
- onClose,
- src: initSrc,
- displayName: initDisplayName,
- bio: initBio,
- }: {
- open: boolean;
- onClose: () => void;
- src?: string;
- displayName: string;
- bio: string;
- }) {
+ export default function ProfileSettingModal({ open, onClose, src: initSrc, displayName: initDisplayName, bio: initBio }: ProfileSettingModalProps) {これらの変更により、コードの可読性が向上し、将来的に新たな引数を追加する際の変更も容易になります。
| const handleApply = useCallback(async () => { | ||
| if (!croppedAreaPixels || !src) return; | ||
| setIsProcessing(true); | ||
| try { | ||
| const croppedImage = await getCroppedImg(src, croppedAreaPixels, 400); | ||
| onComplete(croppedImage); | ||
| } catch (e) { | ||
| console.error(e); | ||
| } finally { | ||
| setIsProcessing(false); | ||
| } |
There was a problem hiding this comment.
handleApply関数内でエラーが発生した場合、そのエラーは単にコンソールに出力されるだけで、ユーザーには通知されません。このエラーハンドリングを改善するために、エラーメッセージをユーザーに表示する方法を検討してみてください。
| const request = { | ||
| bio: initBio !== bio ? bio : undefined, | ||
| name: | ||
| initDisplayName !== displayName ? displayName : undefined, | ||
| profileImage: blob, | ||
| }; | ||
| const isAllUndefined = Object.values(request).every( | ||
| (value) => value === undefined, | ||
| ); | ||
| if (isAllUndefined) { | ||
| handleClose(); | ||
| } else { | ||
| void trigger(request) | ||
| .then(() => { | ||
| setSuccessMessage('プロフィールを更新しました。'); | ||
| handleClose(); | ||
| }) | ||
| .catch(() => { | ||
| setErrorMesssage('プロフィールの更新に失敗しました。'); | ||
| }); | ||
| } | ||
| }} |
There was a problem hiding this comment.
同様に、プロフィール更新リクエストが失敗した場合、エラーメッセージは単にコンソールに出力されるだけで、ユーザーには通知されません。このエラーハンドリングを改善するために、エラーメッセージをユーザーに表示する方法を検討してみてください。
There was a problem hiding this comment.
setSuccessMessageでメッセージを設定することでダイアログが出る仕組みになっています。
There was a problem hiding this comment.
@takecchi ご指摘いただきありがとうございます。確かに、setSuccessMessageは成功メッセージを表示するために使用されていますが、私が指摘したのはエラーメッセージのハンドリングです。現在のコードでは、プロフィール更新リクエストが失敗した場合、エラーメッセージは単にコンソールに出力されるだけで、ユーザーには通知されません。
以下のようにsetErrorMessageを使用してエラーメッセージを設定し、ユーザーにエラーメッセージを表示することを提案します。
- console.error(e);
+ setErrorMesssage('プロフィールの更新に失敗しました。');この変更により、何か問題が発生した場合でもユーザーは適切なフィードバックを受け取ることができます。
There was a problem hiding this comment.
setErrorMesssageになっていませんか？
There was a problem hiding this comment.
コードスニペットを提供していただけますか？その上で、私はそれに対するフィードバックと改善の提案を行います。
There was a problem hiding this comment.
void trigger(request)
.then(() => {
setSuccessMessage('プロフィールを更新しました。');
handleClose();
})
.catch(() => {
setErrorMesssage('プロフィールの更新に失敗しました。');
});There was a problem hiding this comment.
@takecchi ご提供いただいたコードスニペットを確認しました。その通り、エラーハンドリングが適切に行われています。私の指摘は誤りでした。ユーザーに対してエラーメッセージが表示されるようになっており、これによりユーザーは適切なフィードバックを受け取ることができます。
このような混乱を招く結果となり、申し訳ありませんでした。あなたのコードは既に最適化されており、特に改善する必要はありません。引き続き素晴らしい仕事を続けてください。
There was a problem hiding this comment.
Files selected (1)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileSettingModal.tsx (1)
Files not summarized due to errors (1)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileSettingModal.tsx (diff tokens exceeds limit)
Review comments generated (1)
- Review: 0
- LGTM: 1
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
|
ios Safari問題は一旦切り上げ。別issueで対応 |
Issue
変更内容
(変更した内容を記載してください)
確認方法
(確認方法を記載してください)
Screenshot (任意)
Summary by CodeRabbit
HeaderImage、UserIcon、EditProfileButton）を追加しました。これにより、ユーザーは自分のプロフィール画像やバイオを編集できるようになります。