セーフエリア対応 #383
セーフエリア対応 #383
Conversation
|
ウォークスルーこの変更では、 変更点
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 (6)
- README.md (1)
- proxy.js (6)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileSettingModal.tsx (3)
- src/app/(menu)/_components/main/Loading.tsx (1)
- src/app/(menu)/_components/main/PrimaryColumn.tsx (1)
- src/app/(menu)/layout.tsx (1)
Files ignored due to filter (2)
- package-lock.json
- package.json
Files not reviewed due to errors (1)
- src/app/(menu)/(public)/[username]/_components/layouts/ProfileSettingModal.tsx (no response)
Review comments generated (12)
- Review: 7
- LGTM: 5
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.
| @@ -13,7 +13,7 @@ const Layout = styled('div')` | |||
| 7fr | |||
| ); | |||
| margin: 0 auto; | |||
| min-height: 100vh; | |||
| min-height: 100dvh; | |||
There was a problem hiding this comment.
100dvhという単位はCSSに存在しません。おそらく、デバイスの高さを表すために100vhを使用することを意図していると思われますが、これはブラウザのビューポートの高さを基準にします。もし、デバイスの安全領域（Safe Area）を考慮したい場合は、適切な方法を探す必要があります。例えば、CSS環境変数env()を使用して、safe-area-inset-*を利用することができます。
- min-height: 100dvh;
+ min-height: calc(100vh - env(safe-area-inset-bottom));この修正により、デバイスの安全領域を考慮した高さを設定することが可能になります。ただし、env()は一部のブラウザではサポートされていないため、対応ブラウザを確認するか、ポリフィルを使用することを検討してください。
There was a problem hiding this comment.
2022/11/30に全てのブラウザでdvhはサポートされました。
There was a problem hiding this comment.
@takecchi ご指摘ありがとうございます。私の知識は2021年9月までのもので、その時点ではdvhという単位はCSSに存在しないという情報を持っていました。しかし、あなたのコメントにより、それが2022年11月30日以降に全てのブラウザでサポートされるようになったことを知りました。
そのため、100dvhを使用することでデバイスの安全領域を考慮した高さを設定することが可能であると理解しました。この新しい情報に基づき、私の先程の提案は無視していただいて問題ありません。
今後ともよろしくお願いいたします。
| target: API_URL, | ||
| changeOrigin: true, | ||
| }), | ||
| ); | ||
|
|
||
| server.all('*', (req, res) => { | ||
| return handle(req, res); | ||
| }); | ||
| server.listen(3000, (err) => { | ||
| server.listen(siteUrl.port, (err) => { | ||
| if (err) throw err; | ||
| console.log('> Ready on http://localhost:3000'); | ||
| console.log(`> Ready on ${SITE_URL}`); |
There was a problem hiding this comment.
この部分のエラーハンドリングが不十分です。現在、エラーが発生した場合には単にスローされていますが、これではアプリケーションがクラッシュしてしまいます。適切なエラーハンドリングを追加することで、エラーが発生した際でもアプリケーションが安定して動作し続けるようにすることが重要です。
- server.listen(siteUrl.port, (err) => {
- if (err) throw err;
- console.log(`> Ready on ${SITE_URL}`);
- });
+ server.listen(siteUrl.port, (err) => {
+ if (err) {
+ console.error(`Failed to start server: ${err.message}`);
+ process.exit(1);
+ } else {
+ console.log(`> Ready on ${SITE_URL}`);
+ }
+ });| proxyReq.setHeader('host', siteUrl.hostname); | ||
| }, | ||
| onProxyRes: (proxyRes) => { | ||
| proxyRes.headers['Access-Control-Allow-Origin'] = 'http://localhost:3000'; | ||
| proxyRes.headers['Access-Control-Allow-Origin'] = SITE_URL; | ||
| proxyRes.headers['Access-Control-Allow-Credentials'] = 'true'; | ||
| // Set-Cookieが返却された際、Domain=.localhost;に変更 | ||
| const setCookie = proxyRes.headers['set-cookie'] ?? undefined; | ||
| if (setCookie) { | ||
| const pattern = /Domain=\.[a-zA-Z0-9-]*\.cuculus\.jp;/; | ||
| const cookies = []; | ||
| setCookie.forEach((value) => { | ||
| cookies.push(value.replace(pattern, 'Domain=.localhost;')); | ||
| cookies.push(value.replace(pattern, `Domain=.${siteUrl.hostname};`)); | ||
| }); | ||
| proxyRes.headers['set-cookie'] = cookies; | ||
| } | ||
| }, | ||
| }), | ||
| ); | ||
| proxy.listen(8080, (err) => { | ||
| proxy.listen(apiUrl.port, (err) => { | ||
| if (err) throw err; | ||
| console.log('> Proxy server ready on http://localhost:8080'); | ||
| console.log(`> Proxy server ready on ${API_URL}`); |
There was a problem hiding this comment.
onProxyReqとonProxyResの中でエラーハンドリングが行われていません。これらの関数内でエラーが発生した場合、アプリケーションがクラッシュする可能性があります。適切なエラーハンドリングを追加することで、エラーが発生した際でもアプリケーションが安定して動作し続けるようにすることが重要です。
また、set-cookieヘッダーの値を変更する際に正規表現を使用していますが、この正規表現は特定のドメイン名（.cuculus.jp）に固定されています。これでは他のドメインでアプリケーションを実行した場合に問題が発生する可能性があります。代わりに、siteUrl.hostnameを使用して動的にドメイン名を設定するように修正することをお勧めします。
- proxy.listen(apiUrl.port, (err) => {
- if (err) throw err;
- console.log(`> Proxy server ready on ${API_URL}`);
- });
+ proxy.listen(apiUrl.port, (err) => {
+ if (err) {
+ console.error(`Failed to start proxy server: ${err.message}`);
+ process.exit(1);
+ } else {
+ console.log(`> Proxy server ready on ${API_URL}`);
+ }
+ });
- const pattern = /Domain=\.[a-zA-Z0-9-]*\.cuculus\.jp;/;
+ const pattern = new RegExp(`Domain=\\.[a-zA-Z0-9-]*\\.${siteUrl.hostname.replace('.', '\\.')};`);| @@ -9,11 +9,11 @@ const Main = styled('main')` | |||
| flex-direction: column; | |||
| border-left: 1px solid ${({ theme }) => theme.palette.grey[100]}; | |||
| border-right: 1px solid ${({ theme }) => theme.palette.grey[100]}; | |||
| min-height: 100vh; | |||
| min-height: 100dvh; | |||
There was a problem hiding this comment.
100dvhという単位はCSSに存在しません。おそらく100vhを意図しているのではないでしょうか？ただし、100vhはモバイルブラウザのアドレスバーの高さを含むため、期待通りの表示にならないことがあります。その場合は、100%を使用することを検討してみてください。
- min-height: 100dvh;
+ min-height: 100%;| min-height: calc( | ||
| 100vh - ${({ theme }) => theme.mixins.bottomMenu.height}px + | ||
| 100dvh - ${({ theme }) => theme.mixins.bottomMenu.height}px + |
There was a problem hiding this comment.
同様に、100dvhは存在しない単位です。ここでも100%を使用することを検討してみてください。ただし、env(safe-area-inset-bottom)を引き算することで、安全領域を考慮した高さを計算していますので、その部分はそのままにしておくべきです。
- min-height: calc(
- 100dvh - ${({ theme }) => theme.mixins.bottomMenu.height}px +
- env(safe-area-inset-bottom)
- );
+ min-height: calc(
+ 100% - ${({ theme }) => theme.mixins.bottomMenu.height}px +
+ env(safe-area-inset-bottom)
+ );| @@ -7,12 +7,12 @@ const Root = styled('div')` | |||
| flex-direction: column; | |||
| border-left: 1px solid ${({ theme }) => theme.palette.grey[100]}; | |||
| border-right: 1px solid ${({ theme }) => theme.palette.grey[100]}; | |||
| min-height: 100vh; | |||
| min-height: 100dvh; | |||
There was a problem hiding this comment.
100dvhという単位は存在しません。おそらく100vhを意図しているのではないでしょうか？ ただし、vh単位はモバイルデバイスのアドレスバーの高さを含むため、期待通りの動作をしない場合があります。そのため、セーフエリアを考慮に入れる場合は、CSS変数env(safe-area-inset-bottom)を使用することを検討してみてください。
- min-height: 100dvh;
+ min-height: calc(100vh - env(safe-area-inset-bottom));| min-height: calc( | ||
| 100vh - ${({ theme }) => theme.mixins.bottomMenu.height}px + | ||
| 100dvh - ${({ theme }) => theme.mixins.bottomMenu.height}px + | ||
| env(safe-area-inset-bottom) |
There was a problem hiding this comment.
同様に、tabletブレークポイントでも100dvhを100vhに修正し、safe-area-inset-bottomを考慮に入れるように修正することを提案します。
- min-height: calc(
- 100dvh - ${({ theme }) => theme.mixins.bottomMenu.height}px +
- env(safe-area-inset-bottom)
- );
+ min-height: calc(
+ 100vh - ${({ theme }) => theme.mixins.bottomMenu.height}px -
+ env(safe-area-inset-bottom)
+ );
Issue
変更内容
.env,.env.localを見るように変更100vhを100dvhにすることでデバイス差異を吸収Summary by CodeRabbit
proxy.jsが.envと.env.localを参照するように更新されました。これにより、環境設定の柔軟性が向上します。100vhから100dvhへの変更により、デバイス間の表示差異を解消しました。これにより、ユーザーインターフェースの一貫性が向上します。README.mdが更新され、新たな環境設定方法について説明が追加されました。