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

Resolve warning which indicate state changed after widget disposed #125

Merged
merged 10 commits into from
Aug 17, 2023

Conversation

snaoyam
Copy link
Member

@snaoyam snaoyam commented Aug 3, 2023

Changes

  • Fix to change state only on mounted
  • Optimize performance of responsive buttons
    by using ValueNotifier instead of setState to notify widgets that needs rebuild

Issue

To Reproduce

시간표 검색결과 화면 닫기 버튼을 빠르게 눌렀다 뗄 경우 GestureDetector의 onTapDown 함수의 delayed computation에서 실행되는 setState이 IconTextButton가 disposed 된 후에 실행되면서 Warning 메시지를 띄움

문제코드 (responsive_button.dart:256-258) Warning 메세지
image image

Fix Procedure

setState를 mounted된 경우에만 실행하는 것으로 해결 c8e53ab

@override
void setState(e) {
  if(mounted) {
    super.setState(e);
  }
}

@snaoyam snaoyam added the bug Something isn't working label Aug 3, 2023
@snaoyam snaoyam requested a review from sboh1214 August 3, 2023 08:12
@snaoyam snaoyam self-assigned this Aug 3, 2023
@github-actions github-actions bot added the size/l label Aug 3, 2023
@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #125 (7f3c4d7) into main (bdbc426) will increase coverage by 0.45%.
The diff coverage is 61.57%.

❗ Current head 7f3c4d7 differs from pull request most recent head bb9b014. Consider uploading reports for the commit bb9b014 to get more accurate results

@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
+ Coverage   26.91%   27.36%   +0.45%     
==========================================
  Files          80       80              
  Lines        4708     4707       -1     
==========================================
+ Hits         1267     1288      +21     
+ Misses       3441     3419      -22     
Files Changed Coverage Δ
lib/home.dart 0.00% <ø> (ø)
lib/pages/course_detail_page.dart 4.71% <ø> (ø)
lib/pages/course_search_page.dart 83.78% <ø> (ø)
lib/pages/lecture_detail_page.dart 4.91% <ø> (ø)
lib/pages/lecture_search_page.dart 82.66% <ø> (ø)
lib/pages/login_page.dart 46.80% <ø> (ø)
lib/pages/main_page.dart 0.00% <ø> (ø)
lib/pages/settings_page.dart 59.74% <ø> (ø)
lib/pages/timetable_page.dart 4.20% <ø> (ø)
lib/pages/user_page.dart 0.00% <0.00%> (ø)
... and 13 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@snaoyam snaoyam force-pushed the bug@responsive-button branch 2 times, most recently from bac7027 to 588c43f Compare August 7, 2023 18:50
@sboh1214 sboh1214 changed the title Resolve state change after disposed warning Resolve warning which indicate state changed after disposed Aug 17, 2023
@sboh1214 sboh1214 changed the title Resolve warning which indicate state changed after disposed Resolve warning which indicate state changed after widget disposed Aug 17, 2023
Copy link
Contributor

@sboh1214 sboh1214 left a comment

Choose a reason for hiding this comment

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

String 대신 Enum을 활용하여 코드가 더 견고해 진 것 같습니다 수고하셨어요 :)

@sboh1214 sboh1214 merged commit 220fd98 into main Aug 17, 2023
5 of 6 checks passed
@sboh1214 sboh1214 deleted the bug@responsive-button branch August 17, 2023 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/l
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants