Skip to content
This repository has been archived by the owner on Dec 3, 2024. It is now read-only.

Greatly improve "Override Changes" button and UX (fixes #1769) #2063

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public class DeviceActivity extends SyncthingActivity implements View.OnClickLis
private static final String TAG = "DeviceSettingsFragment";
private static final String IS_SHOWING_DISCARD_DIALOG = "DISCARD_FOLDER_DIALOG_STATE";
private static final String IS_SHOWING_COMPRESSION_DIALOG = "COMPRESSION_FOLDER_DIALOG_STATE";
private static final String IS_SHOWING_DELETE_DIALOG = "DELETE_FOLDER_DIALOG_STATE";
private static final String IS_SHOW_DELETE_DIALOG = "DELETE_FOLDER_DIALOG_STATE";
private static final int QR_SCAN_REQUEST_CODE = 777;

private static final List<String> DYNAMIC_ADDRESS = Collections.singletonList("dynamic");
Expand Down Expand Up @@ -170,7 +170,7 @@ private void restoreDialogStates(Bundle savedInstanceState) {
showCompressionDialog();
}

if (savedInstanceState.getBoolean(IS_SHOWING_DELETE_DIALOG)){
if (savedInstanceState.getBoolean(IS_SHOW_DELETE_DIALOG)){
showDeleteDialog();
}

Expand Down Expand Up @@ -220,7 +220,7 @@ public void onSaveInstanceState(Bundle outState) {
outState.putBoolean(IS_SHOWING_COMPRESSION_DIALOG, mCompressionDialog != null && mCompressionDialog.isShowing());
Util.dismissDialogSafe(mCompressionDialog, this);

outState.putBoolean(IS_SHOWING_DELETE_DIALOG, mDeleteDialog != null && mDeleteDialog.isShowing());
outState.putBoolean(IS_SHOW_DELETE_DIALOG, mDeleteDialog != null && mDeleteDialog.isShowing());
Util.dismissDialogSafe(mDeleteDialog, this);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.nutomic.syncthingandroid.databinding.FragmentFolderBinding;
import com.nutomic.syncthingandroid.model.Device;
import com.nutomic.syncthingandroid.model.Folder;
import com.nutomic.syncthingandroid.model.FolderStatus;
import com.nutomic.syncthingandroid.service.Constants;
import com.nutomic.syncthingandroid.service.RestApi;
import com.nutomic.syncthingandroid.service.SyncthingService;
Expand Down Expand Up @@ -65,13 +66,16 @@ public class FolderActivity extends SyncthingActivity
"com.nutomic.syncthingandroid.activities.FolderActivity.FOLDER_ID";
public static final String EXTRA_FOLDER_LABEL =
"com.nutomic.syncthingandroid.activities.FolderActivity.FOLDER_LABEL";
public static final String EXTRA_FOLDER_OVERRIDABLE_CHANGES =
"com.nutomic.syncthingandroid.activities.FolderActivity.FOLDER_OVERRIDABLE_CHANGES";
public static final String EXTRA_DEVICE_ID =
"com.nutomic.syncthingandroid.activities.FolderActivity.DEVICE_ID";

private static final String TAG = "FolderActivity";

private static final String IS_SHOWING_DELETE_DIALOG = "DELETE_FOLDER_DIALOG_STATE";
private static final String IS_SHOW_DELETE_DIALOG = "DELETE_FOLDER_DIALOG_STATE";
private static final String IS_SHOW_DISCARD_DIALOG = "DISCARD_FOLDER_DIALOG_STATE";
private static final String IS_SHOW_OVERRIDE_CHANGES_DIALOG = "OVERRIDE_REMOTES_DIALOG_STATE";

private static final int FILE_VERSIONING_DIALOG_REQUEST = 3454;
private static final int PULL_ORDER_DIALOG_REQUEST = 3455;
Expand All @@ -93,6 +97,7 @@ public class FolderActivity extends SyncthingActivity

private Dialog mDeleteDialog;
private Dialog mDiscardDialog;
private Dialog mOverrideChangesDialog;

private Folder.Versioning mVersioning;

Expand Down Expand Up @@ -148,6 +153,7 @@ public void onCreate(Bundle savedInstanceState) {
findViewById(R.id.pullOrderContainer).setOnClickListener(v -> showPullOrderDialog());
findViewById(R.id.versioningContainer).setOnClickListener(v -> showVersioningDialog());
binding.editIgnores.setOnClickListener(v -> editIgnores());
binding.overrideChangesContainer.setOnClickListener(v -> showOverrideChangesDialog());

if (mIsCreateMode) {
if (savedInstanceState != null) {
Expand All @@ -162,26 +168,39 @@ public void onCreate(Bundle savedInstanceState) {
// Open keyboard on label view in edit mode.
binding.label.requestFocus();
binding.editIgnores.setEnabled(false);
setOverrideChangesContainerEnabled(false);
}
else {
// Prepare edit mode.
binding.id.clearFocus();
binding.id.setFocusable(false);
binding.id.setEnabled(false);
binding.directoryTextView.setEnabled(false);

// overridable remotes button
setOverrideChangesContainerEnabled(
getIntent().getBooleanExtra(EXTRA_FOLDER_OVERRIDABLE_CHANGES, false)
);
}

if (savedInstanceState != null){
if (savedInstanceState.getBoolean(IS_SHOWING_DELETE_DIALOG)){
if (savedInstanceState.getBoolean(IS_SHOW_DELETE_DIALOG)){
showDeleteDialog();
}
}

if (savedInstanceState != null){
if (savedInstanceState.getBoolean(IS_SHOWING_DELETE_DIALOG)){
if (savedInstanceState.getBoolean(IS_SHOW_DELETE_DIALOG)){
showDeleteDialog();
}
}
Comment on lines 186 to 196
Copy link
Member

Choose a reason for hiding this comment

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

Entirely optional, just suggesting it as you are renaming the constant which is also a somewhat unrelated cleanup: You could delete one of the two redundant blobs of code.


if (savedInstanceState != null) {
if (savedInstanceState.getBoolean(IS_SHOW_OVERRIDE_CHANGES_DIALOG)){
showOverrideChangesDialog();
}
}

}

/**
Expand Down Expand Up @@ -304,12 +323,18 @@ public void onPause() {
@Override
protected void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState);
outState.putBoolean(IS_SHOWING_DELETE_DIALOG, mDeleteDialog != null && mDeleteDialog.isShowing());
outState.putBoolean(IS_SHOW_DELETE_DIALOG, mDeleteDialog != null && mDeleteDialog.isShowing());
Util.dismissDialogSafe(mDeleteDialog, this);

outState.putBoolean(IS_SHOW_OVERRIDE_CHANGES_DIALOG, mOverrideChangesDialog != null && mOverrideChangesDialog.isShowing());
Util.dismissDialogSafe(mOverrideChangesDialog, this);

if (mIsCreateMode){
outState.putBoolean(IS_SHOW_DISCARD_DIALOG, mDiscardDialog != null && mDiscardDialog.isShowing());
Util.dismissDialogSafe(mDiscardDialog, this);

outState.putBoolean(IS_SHOW_OVERRIDE_CHANGES_DIALOG, mOverrideChangesDialog != null && mOverrideChangesDialog.isShowing());
Util.dismissDialogSafe(mOverrideChangesDialog, this);
Comment on lines +335 to +337
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed again within this condition? As far as I see it's the exact same function call that already happens uncoditionally before.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, was simply following the existing code... will remove, again.

Copy link
Member

@imsodin imsodin Apr 1, 2024

Choose a reason for hiding this comment

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

Just in case you forgot, as you already removed the code in the other case: It's not removed yet here.
(Also not that it matters, but here I don't see what the existing code is that is doing the same/something similar.)

}
}

Expand Down Expand Up @@ -778,4 +803,33 @@ private void setVersioningDescription(String type, String description) {
binding.versioningType.setText(type);
binding.versioningDescription.setText(description);
}

private void setOverrideChangesContainerEnabled(boolean state) {
binding.overrideChangesContainer.setEnabled(state);
for ( int i = 0; i < binding.overrideChangesContainer.getChildCount(); i++ ) {
binding.overrideChangesContainer.getChildAt(i).setEnabled(state);
}
}

private void showOverrideChangesDialog(){
mOverrideChangesDialog = createOverrideChangesDialog();
mOverrideChangesDialog.show();
}

private Dialog createOverrideChangesDialog(){
return Util.getAlertDialogBuilder(this)
.setIcon(R.drawable.outline_arrow_circle_up_24)
.setTitle(R.string.override_changes)
.setMessage(R.string.override_changes_are_you_sure)
.setNegativeButton(android.R.string.cancel, null)
.setPositiveButton(android.R.string.yes, (dialogInterface, i) -> {
RestApi restApi = getApi();
if (restApi != null) {
restApi.overrideChanges(mFolder.id);
mFolderNeedsToUpdate = true;
}
finish();
})
.create();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.nutomic.syncthingandroid.activities.FolderActivity;
import com.nutomic.syncthingandroid.activities.SyncthingActivity;
import com.nutomic.syncthingandroid.model.Folder;
import com.nutomic.syncthingandroid.model.FolderStatus;
import com.nutomic.syncthingandroid.service.Constants;
import com.nutomic.syncthingandroid.service.RestApi;
import com.nutomic.syncthingandroid.service.SyncthingService;
Expand Down Expand Up @@ -101,9 +102,14 @@ private void updateList() {

@Override
public void onItemClick(AdapterView<?> adapterView, View view, int i, long l) {
Folder folder = mAdapter.getItem(i);
FolderStatus folderStatus = mAdapter.getLocalFolderStatus(folder.id);
boolean overridableChanges = folder.type.equals(Constants.FOLDER_TYPE_SEND_ONLY)
&& folderStatus.isOutOfSync();
Intent intent = new Intent(getActivity(), FolderActivity.class)
.putExtra(FolderActivity.EXTRA_IS_CREATE, false)
.putExtra(FolderActivity.EXTRA_FOLDER_ID, mAdapter.getItem(i).id);
.putExtra(FolderActivity.EXTRA_FOLDER_ID, mAdapter.getItem(i).id)
.putExtra(FolderActivity.EXTRA_FOLDER_OVERRIDABLE_CHANGES, overridableChanges);
startActivity(intent);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,12 @@ public class FolderStatus {
public long version;
public String error;
public String watchError;

public boolean isNeedsItems() {
return needFiles + needDirectories + needSymlinks + needDeletes > 0;
}

public boolean isOutOfSync() {
return state.equals("idle") && isNeedsItems();
}
Comment on lines +31 to +37
Copy link
Member

Choose a reason for hiding this comment

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

Given you aren't using isNeedsItems anymore, I'd prefer if you removed that and folded it into isOutOfSync.

}
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,6 @@ public View getView(int position, View convertView, @NonNull ViewGroup parent) {
Folder folder = getItem(position);
binding.label.setText(TextUtils.isEmpty(folder.label) ? folder.id : folder.label);
binding.directory.setText(folder.path);
binding.override.setOnClickListener(v -> {
// Send "Override changes" through our service to the REST API.
Intent intent = new Intent(mContext, SyncthingService.class)
.putExtra(SyncthingService.EXTRA_FOLDER_ID, folder.id);
intent.setAction(SyncthingService.ACTION_OVERRIDE_CHANGES);
mContext.startService(intent);
});
binding.openFolder.setOnClickListener(v -> {
Intent intent = new Intent(Intent.ACTION_VIEW);
intent.setDataAndType(Uri.fromFile(new File(folder.path)), "resource/folder");
Expand Down Expand Up @@ -94,17 +87,12 @@ private void updateFolderStatusView(ItemFolderListBinding binding, Folder folder
FolderStatus folderStatus = mLocalFolderStatuses.get(folder.id);
if (folderStatus == null) {
binding.items.setVisibility(GONE);
binding.override.setVisibility(GONE);
binding.size.setVisibility(GONE);
setTextOrHide(binding.invalid, folder.invalid);
return;
}

long neededItems = folderStatus.needFiles + folderStatus.needDirectories + folderStatus.needSymlinks + folderStatus.needDeletes;
boolean outOfSync = folderStatus.state.equals("idle") && neededItems > 0;
boolean overrideButtonVisible = folder.type.equals(Constants.FOLDER_TYPE_SEND_ONLY) && outOfSync;
binding.override.setVisibility(overrideButtonVisible ? VISIBLE : GONE);
if (outOfSync) {
if (folderStatus.isOutOfSync()) {
binding.state.setText(mContext.getString(R.string.status_outofsync));
binding.state.setTextColor(ContextCompat.getColor(mContext, R.color.text_red));
} else {
Expand Down Expand Up @@ -172,6 +160,10 @@ public void updateFolderStatus(RestApi api) {
}
}

public FolderStatus getLocalFolderStatus(String folderId) {
return mLocalFolderStatuses.get(folderId);
}

private void onReceiveFolderStatus(String folderId, FolderStatus folderStatus) {
mLocalFolderStatuses.put(folderId, folderStatus);
notifyDataSetChanged();
Expand All @@ -185,5 +177,4 @@ private void setTextOrHide(TextView view, String text) {
view.setVisibility(VISIBLE);
}
}

}
10 changes: 10 additions & 0 deletions app/src/main/res/drawable/outline_arrow_circle_up_24.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24"
android:viewportHeight="24"
android:tint="?attr/colorControlNormal">
<path
android:fillColor="@android:color/white"
android:pathData="M12,20c-4.41,0 -8,-3.59 -8,-8s3.59,-8 8,-8s8,3.59 8,8S16.41,20 12,20M12,22c5.52,0 10,-4.48 10,-10c0,-5.52 -4.48,-10 -10,-10C6.48,2 2,6.48 2,12C2,17.52 6.48,22 12,22L12,22zM11,12l0,4h2l0,-4h3l-4,-4l-4,4H11z"/>
</vector>
55 changes: 55 additions & 0 deletions app/src/main/res/layout/fragment_folder.xml
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,16 @@
android:textAppearance="@style/TextAppearance.AppCompat.Caption"
android:layout_marginBottom="10dp" />

</LinearLayout>

<LinearLayout
android:id="@+id/ignoresContainer"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:background="?selectableItemBackground"
android:orientation="vertical"
android:gravity="center_vertical">

<TextView
android:id="@+id/edit_ignores"
style="@style/Widget.Syncthing.TextView.Label.Details"
Expand All @@ -229,6 +239,51 @@
android:text="@string/ignore_patterns"/>

</LinearLayout>

<LinearLayout
android:id="@+id/actionsContainer"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:background="?selectableItemBackground"
android:orientation="vertical"
android:gravity="center_vertical">

<LinearLayout
android:id="@+id/overrideChangesContainer"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:background="?selectableItemBackground"
android:orientation="vertical"
android:gravity="center_vertical"
android:enabled="false">

<TextView
android:id="@+id/overrideChanges"
style="@style/Widget.Syncthing.TextView.Label.Details"
android:layout_width="match_parent"
android:layout_height="0dp"
android:layout_weight="1"
android:background="@null"
android:checked="false"
android:drawableLeft="@drawable/outline_arrow_circle_up_24"
android:drawableStart="@drawable/outline_arrow_circle_up_24"
android:text="@string/override_changes"
android:enabled="false"
/>

<TextView
android:id="@+id/overrideChangesDescription"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="72dp"
android:layout_marginTop="-10dp"
android:paddingBottom="10dp"
android:textAppearance="@style/TextAppearance.AppCompat.Caption"
android:text="@string/override_changes_description"
android:enabled="false"/>

</LinearLayout>
</LinearLayout>
</LinearLayout>
</ScrollView>

Expand Down
17 changes: 1 addition & 16 deletions app/src/main/res/layout/item_folder_list.xml
Original file line number Diff line number Diff line change
Expand Up @@ -44,26 +44,11 @@
android:ellipsize="end"
android:textAppearance="?textAppearanceListItemSecondary" />

<Button
android:id="@+id/override"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_below="@id/directory"
android:contentDescription="@string/override_changes"
android:paddingEnd="20dp"
android:paddingLeft="20dp"
android:paddingRight="20dp"
android:paddingStart="20dp"
android:text="@string/override_changes"
android:drawableLeft="@android:drawable/ic_menu_upload"
android:drawableStart="@android:drawable/ic_menu_upload"
android:textSize="12sp" />

<TextView
android:id="@+id/items"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_below="@id/override"
android:layout_below="@id/directory"
android:textAppearance="?textAppearanceListItemSecondary" />

<TextView
Expand Down
4 changes: 3 additions & 1 deletion app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@
<item quantity="other">%1$d / %2$d Files</item>
</plurals>

<string name="override_changes">Override changes</string>
<string name="override_changes">Override Changes</string>
<string name="override_changes_description">When configured as \"Send Only\" and changes have been made remotely, you may click here to overwrite the folder content on other devices.</string>
<string name="override_changes_are_you_sure"><b>Warning!</b>\n\nThe folder content on other devices will be overwritten to become identical with this device. Files not present here will be deleted on other devices.\n\nAre you sure you want to continue?</string>

<string name="open_file_manager">Open file manager</string>

Expand Down
Loading