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

Make issue/PR body visible while the rest of the conversation is still loading #1128

Merged
merged 9 commits into from
Nov 12, 2021
2 changes: 1 addition & 1 deletion app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ dependencies {
implementation 'com.caverock:androidsvg-aar:1.4'
implementation 'pl.droidsonroids.gif:android-gif-drawable:1.2.21'
implementation 'org.ocpsoft.prettytime:prettytime:4.0.6.Final'
implementation 'com.github.castorflex.smoothprogressbar:library:1.1.0'
implementation 'com.github.castorflex.smoothprogressbar:library:1.3.0'
implementation 'org.ccil.cowan.tagsoup:tagsoup:1.2.1'
implementation 'com.github.pluscubed:recycler-fast-scroll:3de76812553a77bfd25d3aea0a0af4d96516c3e3@aar'
implementation('com.vdurmont:emoji-java:5.1.1') {
Expand Down
25 changes: 24 additions & 1 deletion app/src/main/java/com/gh4a/fragment/IssueFragmentBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import android.widget.ImageView;
import android.widget.TextView;

import com.gh4a.BaseActivity;
import com.gh4a.Gh4Application;
import com.gh4a.R;
import com.gh4a.ServiceFactory;
Expand Down Expand Up @@ -160,7 +161,17 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container,
@Override
public void onViewCreated(View view, Bundle savedInstanceState) {
super.onViewCreated(view, savedInstanceState);
getBaseActivity().addAppBarOffsetListener(mBottomSheet);
// We want to make the user able to read the issue/PR while the rest of the conversation is still loading
if (mInitialComment == null) {
view.findViewById(R.id.content_container).setVisibility(View.VISIBLE);
}

BaseActivity activity = getBaseActivity();
activity.addAppBarOffsetListener(mBottomSheet);
mBottomSheet.post(() -> {
// Fix an issue where the bottom sheet is initially located outside of the visible screen area
mBottomSheet.resetPeekHeight(activity.getAppBarTotalScrollRange());
});
}

@Override
Expand All @@ -183,6 +194,8 @@ protected void onRecyclerViewInflated(RecyclerView view, LayoutInflater inflater

mListHeaderView = inflater.inflate(R.layout.issue_comment_list_header, view, false);
mAdapter.setHeaderView(mListHeaderView);
View loadingView = inflater.inflate(R.layout.list_loading_view, view, false);
showLoadingIndicator(loadingView);
}

@Override
Expand Down Expand Up @@ -315,6 +328,16 @@ protected void onAddData(RootAdapter<TimelineItem, ?> adapter, List<TimelineItem
}

updateMentionUsers();
removeLoadingIndicator(adapter);
}

private void showLoadingIndicator(View loadingView) {
loadingView.setVisibility(View.VISIBLE);
mAdapter.setFooterView(loadingView, null);
}

private void removeLoadingIndicator(RootAdapter<TimelineItem, ?> adapter) {
adapter.setFooterView(null, null);
}

@Override
Expand Down
6 changes: 4 additions & 2 deletions app/src/main/java/com/gh4a/fragment/LoadingFragmentBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ public void onDestroyView() {
super.onDestroyView();
mContentContainer = null;
mProgress = null;
mProgress = null;
}

@Override
Expand Down Expand Up @@ -129,7 +128,10 @@ private void updateContentVisibility() {
View in = mContentShown ? mContentContainer : mProgress;
if (isResumed()) {
out.startAnimation(AnimationUtils.loadAnimation(getActivity(), android.R.anim.fade_out));
in.startAnimation(AnimationUtils.loadAnimation(getActivity(), android.R.anim.fade_in));
// Prevent UI "flashes" by not unnecessarily animating an element when it's already visible
if (in.getVisibility() != View.VISIBLE) {
in.startAnimation(AnimationUtils.loadAnimation(getActivity(), android.R.anim.fade_in));
}
} else {
in.clearAnimation();
out.clearAnimation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,6 @@ public void onRefresh() {

@Override
protected void bindSpecialViews(View headerView) {
if (!mHasLoadedHeadReference) {
return;
}

PullRequestBranchInfoView branchContainer = headerView.findViewById(R.id.branch_container);
branchContainer.bind(mPullRequest.head(), mPullRequest.base(), mHeadReference);
branchContainer.setVisibility(View.VISIBLE);
Expand Down Expand Up @@ -463,6 +459,10 @@ private void loadCommitStatusesIfOpen(boolean force) {
return;
}

// At this point the status box will display a loading placeholder
CommitStatusBox commitStatusBox = mListHeaderView.findViewById(R.id.commit_status_box);
commitStatusBox.setVisibility(View.VISIBLE);

RepositoryStatusService repoService = ServiceFactory.get(RepositoryStatusService.class, force);
String sha = mPullRequest.head().sha();

Expand Down
13 changes: 7 additions & 6 deletions app/src/main/java/com/gh4a/widget/CommitStatusBox.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageView;
import android.widget.ProgressBar;
import android.widget.TextView;

import com.gh4a.R;
Expand All @@ -21,6 +22,7 @@

public class CommitStatusBox extends LinearLayoutCompat implements View.OnClickListener {
private final ImageView mStatusIcon;
private final ProgressBar mLoadingIndicator;
private final TextView mStatusLabel;
private final ViewGroup mStatusContainer;
private final LayoutInflater mInflater;
Expand All @@ -45,13 +47,12 @@ public CommitStatusBox(Context context, AttributeSet attrs, int defStyleAttr) {
mInflater.inflate(R.layout.commit_status_box, this, true);

mStatusIcon = findViewById(R.id.iv_merge_status_icon);
mLoadingIndicator = findViewById(R.id.status_progress);
mStatusLabel = findViewById(R.id.merge_status_label);
mStatusContainer = findViewById(R.id.merge_commit_status_container);
mSummaryTextView = findViewById(R.id.merge_commit_summary);
mDropDownIcon = findViewById(R.id.drop_down_icon);

mHeader = findViewById(R.id.commit_status_header);
mHeader.setOnClickListener(this);
}

public void fillStatus(List<StatusWrapper> statuses, PullRequest.MergeableState mergableState) {
Expand Down Expand Up @@ -95,9 +96,9 @@ public void fillStatus(List<StatusWrapper> statuses, PullRequest.MergeableState
break;
}

setVisibility(View.VISIBLE);

mLoadingIndicator.setVisibility(GONE);
mStatusIcon.setImageResource(statusIconDrawableResId);
mStatusIcon.setVisibility(VISIBLE);
mStatusLabel.setText(statusLabelResId);

mStatusContainer.removeAllViews();
Expand All @@ -110,7 +111,7 @@ public void fillStatus(List<StatusWrapper> statuses, PullRequest.MergeableState
return;
}

mHeader.setClickable(true);
mHeader.setOnClickListener(this);
mDropDownIcon.setVisibility(View.VISIBLE);
mStatusContainer.setVisibility(View.VISIBLE);

Expand Down Expand Up @@ -159,7 +160,7 @@ public void fillStatus(List<StatusWrapper> statuses, PullRequest.MergeableState
}

setSummaryText(failingCount, pendingCount, successCount);
setStatusesExpanded(failingCount + pendingCount > 0);
setStatusesExpanded(false);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/gh4a/widget/OverviewRow.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public OverviewRow(Context context, AttributeSet attrs, int defStyle) {
mIcon = findViewById(R.id.icon);
mLabel = findViewById(R.id.label);
mRedirectNotice = findViewById(R.id.forward_notice);
mProgress = findViewById(R.id.progress);
mProgress = findViewById(R.id.progress_indicator);

final TypedArray a = getContext().obtainStyledAttributes(
attrs, R.styleable.OverviewRow, defStyle, 0);
Expand Down
18 changes: 15 additions & 3 deletions app/src/main/res/layout/commit_status_box.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@
android:layout_height="wrap_content"
android:background="?selectableItemBackground">

<ProgressBar
android:id="@+id/status_progress"
android:layout_width="30dp"
android:layout_height="30dp"
android:layout_marginLeft="@dimen/content_padding"
android:layout_marginRight="10dp"
android:layout_marginTop="@dimen/content_padding"
android:indeterminate="true" />

<ImageView
android:id="@+id/iv_merge_status_icon"
android:layout_width="30dp"
Expand All @@ -21,6 +30,7 @@
android:layout_marginLeft="@dimen/content_padding"
android:layout_marginRight="10dp"
android:layout_marginTop="@dimen/content_padding"
android:visibility="invisible"
tools:src="@drawable/pull_request_merge_ok" />

<com.gh4a.widget.StyleableTextView
Expand All @@ -30,8 +40,8 @@
android:layout_marginTop="@dimen/content_padding"
android:layout_toLeftOf="@+id/drop_down_icon"
android:layout_toRightOf="@id/iv_merge_status_icon"
android:textAppearance="@style/TextAppearance.AppCompat.Body1"
tools:text="All checks have passed" />
android:text="@string/pull_merge_status_loading"
android:textAppearance="@style/TextAppearance.AppCompat.Body1" />

<com.gh4a.widget.StyleableTextView
android:id="@+id/merge_commit_summary"
Expand All @@ -56,7 +66,9 @@
android:layout_marginRight="@dimen/content_padding"
android:layout_marginTop="@dimen/content_padding"
android:scaleType="center"
android:src="@drawable/drop_up_arrow" />
android:visibility="gone"
android:src="@drawable/drop_up_arrow"
tools:visibility="visible"/>

</RelativeLayout>

Expand Down
10 changes: 5 additions & 5 deletions app/src/main/res/layout/loading_fragment.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
android:layout_width="match_parent"
android:layout_height="match_parent">

<FrameLayout
android:id="@+id/content_container"
android:layout_width="match_parent"
android:layout_height="match_parent" />

<fr.castorflex.android.smoothprogressbar.SmoothProgressBar
android:id="@+id/progress"
android:layout_width="match_parent"
Expand All @@ -18,9 +23,4 @@
app:spb_sections_count="6"
app:spb_speed="1.0" />

<FrameLayout
android:id="@+id/content_container"
android:layout_width="match_parent"
android:layout_height="match_parent" />

</FrameLayout>
2 changes: 1 addition & 1 deletion app/src/main/res/layout/overview_row.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
tools:src="@drawable/icon_watch" />

<ProgressBar
android:id="@+id/progress"
android:id="@+id/progress_indicator"
style="@style/Widget.AppCompat.ProgressBar"
android:layout_width="24dp"
android:layout_height="24dp"
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@
<string name="pull_merge_title_description">Merge commit title</string>
<string name="pull_merge_message_description">Merge commit details</string>
<string name="pull_merge_message_hint">If you want to add more details to the merge commit, enter them here.</string>
<string name="pull_merge_status_loading">Loading pull request status…</string>
<string name="pull_merge_status_behind">This branch is out-of-date with the base branch</string>
<string name="pull_merge_status_blocked">Merging is blocked</string>
<string name="pull_merge_status_clean">All checks have passed</string>
Expand Down