fix(widget): resolve ProgressLayout content visibility not restored#1021
Merged
Conversation
…fter loading The ProgressLayout migrated from a third-party library would show the loading spinner but never restore content visibility. Root cause was two-fold: 1. Callers (8 instances across 5 files) called showLoading() before showError() but guarded showError() behind a null-drawable check. When the drawable was null, showError() was skipped and the widget stayed permanently in LOADING state. 2. Content child visibility was reverted by child layout passes after applyState() set them to VISIBLE. Fix: use post-guard to re-assert visibility after layout completes. Changes: - ProgressLayout: direct map assignment + post safety net in applyState - FragmentBaseList/BaseComment/ChannelBase: remove showLoading + guard - BottomSheetList/BottomSheetListUsers: remove showLoading + guard - Add instrumentation tests (13 test cases) for ProgressLayout state machine, visibility restoration, and regression scenarios
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Documentation | 12 minor |
| Complexity | 3 medium |
🟢 Metrics 30 complexity · 19 duplication
Metric Results Complexity 30 Duplication 19
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…th LinearLayout - Reduce widget internal padding from 24dp (spacing_small) to 8dp (lg_margin) for CommentWidget, StatusEditWidget, UsersWidget, FavouriteWidget, and StatusDeleteWidget - Replace FrameLayout with horizontal LinearLayout + Space(weight=1) in adapter_feed_status.xml, adapter_feed_progress.xml, and adapter_feed_message.xml to prevent UsersWidget and action button row from overlapping when multiple buttons are visible - Verified on all 3 Status Feed tabs with all 5 widgets visible
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The ProgressLayout migrated from a third-party library would show the loading spinner but never restore content visibility. This was caused by two issues:
1. Caller-level: Null-drawable guards leaving state stuck in LOADING
8 call sites across 5 files called
showLoading()beforeshowError()but guardedshowError()behind anif (drawable != null)check. When the drawable was null (e.g., missing context or stripped resource),showError()was skipped, leaving the widget permanently in LOADING state.Fix: Remove the preemptive
showLoading()call and the null-drawable guard. Pass the nullable drawable directly tostateLayout.showError(), which already handles null drawables correctly.Files fixed:
base/custom/sheet/BottomSheetList.kt—showError,showEmptyview/sheet/BottomSheetListUsers.kt—showError,showEmptybase/custom/fragment/FragmentBaseList.kt—showError,showEmptybase/custom/fragment/FragmentBaseComment.kt—showError,showEmpty(+ removed spuriousshowLoading()outside condition)base/custom/fragment/FragmentChannelBase.kt—showError,showEmpty2. Widget-level: Content visibility reverted by child layout passes
applyState()correctly set content children to VISIBLE, but child Views (CustomSwipeRefreshLayout, RecyclerView) reverted their visibility during their own layout pass. The fix adds apost { }safety net that re-checks and re-asserts content visibility after layout completes.Fix in
ProgressLayout.kt:putIfAbsent→ direct[child] =assignment (always saves current visibility)child.visibility = saved ?: View.VISIBLE(fallback for untracked children)post { }re-check after state transitions to CONTENTInstrumentation tests added
13 test cases covering state transitions, error handling, null drawables, multi-child visibility, multiple state cycles, and synchronous visibility restoration.
Types of changes