-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: implement lottie #539
Conversation
📝 Walkthrough📝 WalkthroughWalkthrough이 풀 리퀘스트에서는 Lottie 애니메이션을 사용하여 UI의 로딩 인디케이터를 개선하는 여러 변경 사항이 포함되어 있습니다. 새로운 JSON 파일 Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (9)
lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart (1)
33-35
: 로딩 애니메이션의 크기를 반응형으로 개선하는 것이 좋겠습니다.현재 구현된 고정 크기(80x80)는 다양한 화면 크기에서 일관된 사용자 경험을 제공하지 못할 수 있습니다.
다음과 같이 MediaQuery를 사용하여 화면 크기에 따라 동적으로 조절되도록 개선해보세요:
-return Center( - child: Lottie.asset(Assets.lotties.loading, - height: 80, width: 80)); +return Center( + child: Lottie.asset( + Assets.lotties.loading, + height: MediaQuery.of(context).size.width * 0.2, + width: MediaQuery.of(context).size.width * 0.2, + ), +);lib/app/modules/notices/presentation/pages/detail_page.dart (1)
55-56
: 로딩 애니메이션 에러 처리 개선이 필요합니다Lottie 애니메이션이 잘 구현되었지만, 애니메이션 에셋 로딩 실패 시 대체 UI를 제공하는 것이 좋습니다.
다음과 같이 개선하는 것을 추천드립니다:
- return Center( - child: Lottie.asset(Assets.lotties.loading, height: 80, width: 80)); + return Center( + child: Lottie.asset( + Assets.lotties.loading, + height: 80, + width: 80, + errorBuilder: (context, error, stackTrace) { + return const CircularProgressIndicator(); + }, + ), + );pubspec.yaml (1)
60-60
: Lottie 패키지 버전 업데이트 권장현재 ^3.1.3 버전을 사용하고 계신데, 최신 안정 버전인 3.1.4로 업데이트하시는 것을 권장드립니다. 보안 패치와 버그 수정이 포함되어 있을 수 있습니다.
- lottie: ^3.1.3 + lottie: ^3.1.4lib/app/modules/user/presentation/pages/packages_page.dart (1)
48-50
: 로딩 애니메이션 구현 개선 제안현재 구현은 작동하지만 다음 사항들을 고려해보시기 바랍니다:
- 고정된 크기(80x80) 대신 화면 크기에 따라 반응형으로 조정되는 것이 좋습니다.
- 애니메이션 에셋 로딩 실패에 대한 폴백(fallback) 처리가 필요합니다.
- 저사양 기기에서의 성능을 고려하여 애니메이션 최적화가 필요할 수 있습니다.
다음과 같이 개선해보시는 건 어떨까요:
return Center( - child: Lottie.asset(Assets.lotties.loading, - height: 80, width: 80)); + child: LayoutBuilder( + builder: (context, constraints) { + final size = constraints.maxWidth * 0.2; // 화면 너비의 20% + return Lottie.asset( + Assets.lotties.loading, + height: size, + width: size, + errorBuilder: (context, error, stackTrace) { + return const CircularProgressIndicator(); + }, + frameRate: FrameRate.max(30), // 성능 최적화 + ); + }, + ), );lib/app/modules/notices/presentation/widgets/editor.dart (1)
79-81
: 애니메이션 크기를 반응형으로 개선하는 것을 고려해보세요현재 하드코딩된 80x80 크기 대신, 화면 크기에 따라 동적으로 조절되는 크기를 사용하는 것이 좋습니다.
다음과 같이 MediaQuery를 사용하여 개선할 수 있습니다:
- child: Lottie.asset(Assets.lotties.loading, - height: 80, width: 80), + child: Lottie.asset( + Assets.lotties.loading, + height: MediaQuery.of(context).size.width * 0.2, + width: MediaQuery.of(context).size.width * 0.2, + ),lib/app/modules/common/presentation/widgets/ziggle_button.dart (1)
148-149
: 로딩 애니메이션 크기를 버튼 타입에 맞게 조정하는 것이 좋겠습니다.현재 Lottie 애니메이션이 모든 버튼 타입에 대해 고정된 크기(30x30)를 사용하고 있습니다. 버튼의 크기에 따라 애니메이션 크기도 조정되어야 더 자연스러울 것 같습니다.
다음과 같이 버튼 타입별로 애니메이션 크기를 다르게 지정하는 것을 제안드립니다:
- child: Lottie.asset(Assets.lotties.loading, - width: 30, height: 30), + child: Lottie.asset( + Assets.lotties.loading, + width: type == ZiggleButtonType.small ? 20 : + type == ZiggleButtonType.big ? 35 : 30, + height: type == ZiggleButtonType.small ? 20 : + type == ZiggleButtonType.big ? 35 : 30, + ),lib/app/modules/notices/presentation/widgets/list_layout.dart (1)
32-34
: 매직 넘버를 상수로 추출하는 것이 좋습니다하드코딩된 크기 값(80)을 의미 있는 상수로 추출하면 유지보수성이 향상될 것 같습니다.
다음과 같이 변경하는 것을 제안합니다:
+ static const double _loadingAnimationSize = 80.0; + @override Widget build(BuildContext context) { return BlocBuilder<NoticeListBloc, NoticeListState>( builder: (context, state) { return RefreshIndicator( onRefresh: () => NoticeListBloc.refresh(context), child: state.showLoading ? Center( child: Lottie.asset(Assets.lotties.loading, - height: 80, width: 80), + height: _loadingAnimationSize, + width: _loadingAnimationSize),lib/app/modules/groups/presentation/pages/group_management_main_page.dart (1)
88-89
: 로딩 애니메이션의 반응형 크기 조정 고려현재 Lottie 애니메이션이 고정된 크기(80x80)로 설정되어 있습니다. 다양한 화면 크기에서 더 나은 사용자 경험을 제공하기 위해 반응형 크기 조정을 고려해보세요.
다음과 같이 MediaQuery를 사용하여 화면 크기에 따라 동적으로 조정되도록 수정할 수 있습니다:
-Lottie.asset(Assets.lotties.loading, height: 80, width: 80) +Lottie.asset( + Assets.lotties.loading, + height: MediaQuery.of(context).size.height * 0.1, + width: MediaQuery.of(context).size.width * 0.1, +)lib/app/modules/notices/presentation/pages/search_page.dart (1)
146-148
: 로딩 애니메이션 크기를 상수로 분리하는 것을 고려해보세요.하드코딩된 크기 값(80x80)을 상수로 분리하면 재사용성과 일관성을 향상시킬 수 있습니다.
다음과 같이 변경하는 것을 제안합니다:
+ static const double _loadingAnimationSize = 80.0; ... child: Lottie.asset(Assets.lotties.loading, - height: 80, width: 80), + height: _loadingAnimationSize, + width: _loadingAnimationSize),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pubspec.lock
is excluded by!**/*.lock
📒 Files selected for processing (13)
assets/lotties/complete.json
(1 hunks)assets/lotties/loading.json
(1 hunks)lib/app/modules/common/presentation/widgets/ziggle_button.dart
(2 hunks)lib/app/modules/groups/presentation/pages/group_creation_done_page.dart
(2 hunks)lib/app/modules/groups/presentation/pages/group_management_main_page.dart
(2 hunks)lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart
(2 hunks)lib/app/modules/notices/presentation/pages/detail_page.dart
(2 hunks)lib/app/modules/notices/presentation/pages/search_page.dart
(3 hunks)lib/app/modules/notices/presentation/widgets/editor.dart
(2 hunks)lib/app/modules/notices/presentation/widgets/list_layout.dart
(4 hunks)lib/app/modules/user/presentation/pages/packages_page.dart
(2 hunks)lib/app/modules/user/presentation/pages/profile_page.dart
(2 hunks)pubspec.yaml
(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- assets/lotties/complete.json
- assets/lotties/loading.json
🔇 Additional comments (12)
lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart (1)
4-4
: 임포트 구문이 적절히 추가되었습니다!
Lottie 애니메이션 구현을 위한 필수 패키지들이 올바르게 임포트되었습니다.
Also applies to: 9-9
lib/app/modules/notices/presentation/pages/detail_page.dart (1)
4-4
: 필요한 패키지들이 올바르게 임포트되었습니다!
Lottie 애니메이션 구현에 필요한 패키지와 에셋이 적절하게 추가되었습니다.
Also applies to: 12-12
pubspec.yaml (1)
90-90
: 애셋 경로 추가 확인됨
Lottie 애니메이션 파일들을 위한 assets/lotties/ 경로가 올바르게 추가되었습니다. CircularProgressIndicator를 Lottie 애니메이션으로 교체하기 위한 준비가 잘 되어있습니다.
lib/app/modules/user/presentation/pages/packages_page.dart (1)
5-5
: LGTM!
Lottie 패키지 import가 적절하게 추가되었습니다.
lib/app/modules/notices/presentation/widgets/editor.dart (1)
3-3
: LGTM!
Lottie 패키지 import가 적절하게 추가되었습니다.
lib/app/modules/groups/presentation/pages/group_creation_done_page.dart (1)
3-3
: Lottie 패키지 의존성 확인 필요
pubspec.yaml에 올바른 버전의 lottie 패키지가 추가되어 있는지 확인이 필요합니다.
✅ Verification successful
Lottie 패키지 및 애셋 설정이 올바르게 되어있음
pubspec.yaml에 Lottie 패키지가 정상적으로 추가되어 있으며(v3.1.3), assets/lotties/complete.json 파일이 존재하고 코드에서 올바르게 사용되고 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Lottie package dependency in pubspec.yaml
cat pubspec.yaml | grep -A 2 "lottie:"
Length of output: 74
Script:
#!/bin/bash
# Check if Lottie assets exist in the repository
fd -e json -e lottie
# Check the actual usage of Lottie in the file
rg "Lottie\." lib/app/modules/groups/presentation/pages/group_creation_done_page.dart -A 2
Length of output: 645
lib/app/modules/common/presentation/widgets/ziggle_button.dart (1)
2-2
: 새로운 패키지 임포트가 적절히 추가되었습니다!
Lottie 애니메이션과 자산 생성기를 위한 임포트가 올바르게 추가되었습니다.
Also applies to: 5-5
lib/app/modules/notices/presentation/widgets/list_layout.dart (1)
3-3
: 새로운 의존성 import가 적절히 추가되었습니다
Lottie 애니메이션 구현을 위한 필수 패키지와 에셋 import가 올바르게 추가되었습니다.
Also applies to: 17-17
lib/app/modules/groups/presentation/pages/group_management_main_page.dart (1)
4-4
: Lottie 패키지 import가 올바르게 추가되었습니다.
패키지 import 섹션에 적절하게 배치되어 있습니다.
lib/app/modules/user/presentation/pages/profile_page.dart (2)
6-6
: Lottie 패키지 import가 적절히 추가되었습니다.
새로운 애니메이션 기능을 위한 필수 패키지가 올바르게 import 되었습니다.
80-82
: 로딩 인디케이터가 성공적으로 Lottie 애니메이션으로 교체되었습니다.
기존의 CircularProgressIndicator를 Lottie 애니메이션으로 교체한 것이 적절합니다. 크기 제약(60x60)도 UI에 알맞게 설정되었습니다.
다만, 애니메이션이 무한 반복되는지 확인이 필요합니다.
애니메이션 반복 설정을 확인하기 위해 다음 스크립트를 실행하세요:
✅ Verification successful
Let me analyze the Lottie animation JSON file. I can see that:
fr
(frame rate) is set to 60ip
(in-point) is set to 0op
(out-point) is set to 114/115 for different layers- The animation has a clear start and end point with defined keyframes
Let me check how the Lottie animation is being used in the code:
로딩 애니메이션이 적절하게 구현되어 있습니다.
Lottie 애니메이션 파일을 분석한 결과, 애니메이션이 0프레임에서 시작하여 114/115프레임에서 종료되도록 설정되어 있으며, 60fps로 실행됩니다. Flutter의 Lottie 위젯은 기본적으로 애니메이션을 무한 반복하므로 로딩 인디케이터로서 적절하게 동작할 것입니다.
또한 이 로딩 애니메이션이 앱 전반에 걸쳐 일관되게 사용되고 있어 UI/UX 측면에서도 바람직합니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the loading animation JSON has repeat property set
# Check the loading.json file for repeat settings
rg -A 5 '"op"|"ip"|"fr"' assets/lotties/loading.json
Length of output: 3145
Script:
#!/bin/bash
# Check how Lottie.asset is being used
rg -A 2 "Lottie\.asset" lib/
Length of output: 3871
lib/app/modules/notices/presentation/pages/search_page.dart (1)
5-5
: LGTM!
Lottie 패키지가 올바르게 임포트되었습니다.
lib/app/modules/groups/presentation/pages/group_creation_done_page.dart
Outdated
Show resolved
Hide resolved
@GwanLiZa 리뷰 해줘잉 |
8513569
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
lib/app/modules/user/presentation/pages/profile_page.dart (1)
80-83
: 로딩 애니메이션 구현에 대한 개선 제안현재 구현은 작동하지만 다음과 같은 개선사항을 고려해보시기 바랍니다:
- 크기 계산에 사용된 0.15는 상수로 추출하는 것이 좋습니다.
- 큰 화면에서는 애니메이션이 너무 작을 수 있으므로, 최소/최대 크기 제한을 추가하는 것이 좋습니다.
- 애니메이션 에셋 로딩 실패에 대한 폴백(fallback) 처리가 필요합니다.
다음과 같이 개선해보세요:
+ // 상수 정의 + const double _loadingAnimationSizeRatio = 0.15; + const double _minLoadingSize = 40.0; + const double _maxLoadingSize = 80.0; Center( child: Lottie.asset( Assets.lotties.loading, - height: MediaQuery.of(context).size.width * 0.15, - width: MediaQuery.of(context).size.width * 0.15, + height: (MediaQuery.of(context).size.width * _loadingAnimationSizeRatio) + .clamp(_minLoadingSize, _maxLoadingSize), + width: (MediaQuery.of(context).size.width * _loadingAnimationSizeRatio) + .clamp(_minLoadingSize, _maxLoadingSize), + errorBuilder: (context, error, stackTrace) => + const CircularProgressIndicator(), ), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
lib/app/modules/groups/presentation/pages/group_management_main_page.dart
(2 hunks)lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart
(2 hunks)lib/app/modules/notices/presentation/pages/search_page.dart
(3 hunks)lib/app/modules/notices/presentation/widgets/list_layout.dart
(4 hunks)lib/app/modules/user/presentation/pages/packages_page.dart
(2 hunks)lib/app/modules/user/presentation/pages/profile_page.dart
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- lib/app/modules/groups/presentation/pages/group_management_main_page.dart
- lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart
- lib/app/modules/notices/presentation/pages/search_page.dart
- lib/app/modules/notices/presentation/widgets/list_layout.dart
- lib/app/modules/user/presentation/pages/packages_page.dart
🔇 Additional comments (1)
lib/app/modules/user/presentation/pages/profile_page.dart (1)
6-6
: LGTM! Lottie 패키지 임포트가 올바르게 추가되었습니다.
Dart 임포트 컨벤션을 잘 따르고 있습니다.
Summary by CodeRabbit