fix: bug causing permanent CircularProgressIndicator when referenced board is deleted fixes #2001#2011
fix: bug causing permanent CircularProgressIndicator when referenced board is deleted fixes #2001#2011sshdopey wants to merge 4 commits intoAppFlowy-IO:mainfrom
Conversation
|
|
annieappflowy
added
grid
a-wallen
left a comment
a-wallen
left a comment
There was a problem hiding this comment.
Looks good! Thanks for the quick turnaround. See my comments and address them as you see fit.
...nd/appflowy_flutter/lib/plugins/document/presentation/plugins/base/built_in_page_widget.dart
Show resolved
Hide resolved
Co-authored-by: Alex Wallen <wallenstephen@outlook.com>
|
Thanks for the correction, I learnt something from that stackoverflow anwer. |
sshdopey
changed the title
|
All checks completed. I hope it's okay now. I still am open for advice and correction |
|
@DestinedCodes do you think that you can add a test for this PR? FYI it looks good to me, but a test would be nice so that the codebase doesn't regress. |
|
TBH, I have never written a test case before now. I only came across this issue as a github octern applicant. The skills i have learnt and honed during the last and current week are something I'll always be grateful for. YES I think I can add a test case for the PR.😊 |
|
I just worked on an integration test that uses commands from the slash menu. I think you might want to look at this file to help create your test frontend/appflowy_flutter/integration_test/slash_command_test.dart. |
|
fixes #2001 |
|
I am still working on the test, I am not done yet. |
...nd/appflowy_flutter/lib/plugins/document/presentation/plugins/base/built_in_page_widget.dart
Outdated
Show resolved
Hide resolved
| if (board != null) { | ||
| return _build(context, board); | ||
| } | ||
| return const SizedBox.shrink(); |
There was a problem hiding this comment.
I'm not sure I'm a fan of this solution.
I would prefer that deleting a Page would also remove it from any documents which might reference it.
There was a problem hiding this comment.
IIRC, the backend can either return a board, null, or error so the widget still needs to account for this case. This solution will do for now.
Co-authored-by: Mathias Mogensen <42929161+Xazin@users.noreply.github.com>
|
@DestinedCodes, we still want this change! Please reach out on Discord if you need help with the test! |
|
Hey @DestinedCodes, if you need help with the tests let us know, I can help write some tests if you want, and open a PR to your branch on your fork. |
5 participants
If you add a referenced board or grid to a document and delete the board, the document will have a permanent circular progress indicator.
This pull request fixes a bug where deleting a referenced board results in a permanent circular progress indicator on the page. The issue occurs because the app is not handling the case where the referenced board is deleted.
To fix this issue, I added a check for whether the referenced board is null. If it is null, I returned
SizedBox.shrink()to remove the board from the page. Additionally, I added a check for if theFutureBuilderhas an error, and returnedSizedBox.shrink()in that case as well.I tested this fix by creating a new board, referencing it on a page, deleting the board, closing and reopening the app, and checking that the circular progress indicator was no longer present.
Please review and merge this pull request.
Thank you!