Skip to content
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

Custom RefreshIndicator 생성 #202

Closed
wants to merge 7 commits into from

Conversation

thomaskim1130
Copy link
Contributor

@thomaskim1130 thomaskim1130 commented May 20, 2024

Overview

로딩을 위해 pull down하는 거리 감소. 편의성 증진

Changes

  • refreshIndicator.adaptive()를 ScrollNotification 클래스를 활용해 custom화 함.
  • 기존 pull down 50(변경 불가) 에서 30으로 조정.
  • main_page.dart, post_list_show_page.dart, user_page.dart, post_view_page 등 페이지에 적용

Implementaion Method

  • global_key.dart에 refreshIndicatorKey 추가 (페이지별로 개별 생성)
  • utils/refresh_indicator.dart를 추가하여 메소드 customRefreshIndicator 생성

After Changes

  • scrollDownLength를 customize할 수 있음

Related Issues

Rollback Scenario

  1. customRefreshIndicator 대신 refreshIndicator.adaptive 사용
  2. globalKey 모두 제거
  3. AlwaysScrollablePhysics의 parent 변경하기

TODO

  • user_view_page.dart 에도 적용. (DONE)
  • refreshIndicator 다수 중복. Wrapper 함수 제작? (DONE)

refreshIndicatorKey (gloabl_key.dart 내부에도 저장) 를 생성해 trigger를 위해 pull down해야 하는 거리를 조정함 (50 -> 20).

위를 main_page, user_page, post_list_show_page 에만 적용함.
@thomaskim1130 thomaskim1130 added the enhancement New feature or request label May 20, 2024
@thomaskim1130 thomaskim1130 added this to the v1.2.0 milestone May 20, 2024
@thomaskim1130 thomaskim1130 requested a review from sangohkim May 20, 2024 16:44
@thomaskim1130 thomaskim1130 self-assigned this May 20, 2024
@thomaskim1130 thomaskim1130 linked an issue May 20, 2024 that may be closed by this pull request
@thomaskim1130 thomaskim1130 changed the title Feat(global_key.dart): Custom RefreshIndicator 생성 Custom RefreshIndicator 생성 May 27, 2024
@thomaskim1130 thomaskim1130 marked this pull request as ready for review July 11, 2024 12:16
@thomaskim1130 thomaskim1130 marked this pull request as draft July 11, 2024 12:45
GlobalKey 'refreshIndicatorKey'를 페이지별로 생성하여 duplicate global key 이슈를 방지
추후 다른 파일에도 유사하게 적용할 예정
Android에서 ScrollNotification이 negative일 때 (즉 위에서 아래로 outOfRange로 스크롤 할 때) 의 pixel을 잡지 못하는 상황을 개선함.
구체적으로, parent: BouncingScrollPhysics()를 AlwaysScrollableScrollPhysics에 넣어 android도 ios와 동일한 설정으로 바꿈.
기존의 RefreshIndicator.adaptive를 계승하는 customRefreshIndicator 메소드를 정의함 (utils/refresh_indicator.dart).
Wrapper 함수 형식으로 생성하여 공식 문서상 parameter를 모두 가져옴.
customRefreshIndicator를 main_page, notification_page, post_list_show_page, post_view_page, user_page, user_view_page 에 모두 적용함.
@thomaskim1130 thomaskim1130 marked this pull request as ready for review July 13, 2024 19:13
@@ -0,0 +1,71 @@
import 'package:flutter/material.dart';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위젯을 생성하는 기능이니 utils 보다는 widgets 디렉터리에 위치시키는게 좋을것 같습니다.

@@ -4,3 +4,7 @@ import 'package:flutter/material.dart';
/// globalKey를 설정합니다.
final GlobalKey<ScaffoldMessengerState> snackBarKey =
GlobalKey<ScaffoldMessengerState>();

/// refreshIndicator를 custom하게 변경하는 globalKey
final GlobalKey<RefreshIndicatorState> refreshIndicatorKey =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 파일에 있는 refreshIndicatorKey는 UserPage에서만 사용하고, 다른 페이지에서는 페이지 내부에서 _refreshIndicatorKey를 사용하는 방식으로 구현되어 있는데 사용방식의 통일이 필요할 것 같습니다.

),
),
);
return NotificationListener<ScrollNotification>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

customRefreshIndicator가 이미 NotificationListener를 반환하는 형태로 구현되어 있어 수정이 필요할 것 같습니다. customRefreshIndicator의 적용방식이 UserPage에서만 조금씩다른 것 같은데(global key 사용 및 현재 부분에서) 혹시 다르게 구현하신 이유가 있을까요?

Copy link
Collaborator

@sangohkim sangohkim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RefreshIndicator 직접 만드시느라 수고많으셨습니다! 리뷰 완료하였고 코멘트 달린 부분들 한번 봐주시면 될 것 같아요.
그리고 한가지 수정해야할 부분이 보이는데 PostListShowPage, NotificationPage에서 pull down을 조금만 하면 의도했던 방식으로 리프레시가 진행되지만 pull down을 많이 하게되는 경우 페이지 중앙의 로딩인디케이터(저희가 저번에 페이지가 처음 로드될때만 보여주는걸로 한 부분)이 다시 나타나는 현상을 확인하였습니다. 저도 시간날때 원인을 파악해볼테니 이 부분도 확인부탁드립니다.

  • iOS, android 모두에서 확인되었습니다

refresh_indicator.dart를 lib/widgets로 이동
user_page.dart에서 중복 implementation 제거
@thomaskim1130 thomaskim1130 requested a review from sangohkim July 24, 2024 20:28
notificationPage, postListShowPage, postViewPage, userPage, userViewPage에서
isLoading 등 관련 변수들을 false 처리하면서 LoadingIndicator가 오직 처음에만 나타나도록 함.

userPage에서는 특히 TabType별로 global key를 분리하여 에러를 방지함.
@sangohkim sangohkim removed this from the v1.2.0 milestone Aug 29, 2024
@thomaskim1130
Copy link
Contributor Author

Closed for now (outdated, continued in #246)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]LoadingIndicator 에 필요한 간격 줄이기
2 participants