-
-
Notifications
You must be signed in to change notification settings - Fork 386
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
Add ability to easily assign groups #1497
Add ability to easily assign groups #1497
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally this looks good to me! Couple of comments though.
|
||
_selectedVaultGroupModel = new VaultGroupModel(group); | ||
} else { | ||
newGroupText.setError(getString(R.string.error_required_field)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
app/src/main/java/com/beemdevelopment/aegis/ui/MainActivity.java
Outdated
Show resolved
Hide resolved
@@ -95,6 +101,7 @@ public class MainActivity extends AegisActivity implements EntryListView.Listene | |||
private String _pendingSearchQuery; | |||
|
|||
private List<VaultEntry> _selectedEntries; | |||
private VaultGroupModel _selectedVaultGroupModel; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be moved to the scope of startAssignGroupsDialog
.
app/src/main/res/values/strings.xml
Outdated
@@ -208,6 +208,9 @@ | |||
<string name="edit">Edit</string> | |||
<string name="select_all">Select all</string> | |||
<string name="assign_icons">Assign icons</string> | |||
<string name="assign_groups">Assign group</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: "Assign to group"
app/src/main/res/values/strings.xml
Outdated
@@ -208,6 +208,9 @@ | |||
<string name="edit">Edit</string> | |||
<string name="select_all">Select all</string> | |||
<string name="assign_icons">Assign icons</string> | |||
<string name="assign_groups">Assign group</string> | |||
<string name="assign_groups_dialog_summary">Select a group that you wish to add to the selected entries.</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: "Select a group you wish to assign the selected entries to."
|
||
<com.google.android.material.textfield.TextInputLayout | ||
android:id="@+id/text_group_name_layout" | ||
android:hint="@string/set_group" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest changing this to @string/group_name_hint
.
ArrayList<VaultGroupModel> groupsArray = new ArrayList<>(); | ||
groupsArray.add(new VaultGroupModel(getString(R.string.new_group))); | ||
groupsArray.addAll(groups.stream().map(VaultGroupModel::new).collect(Collectors.toList())); | ||
|
||
ArrayAdapter<VaultGroupModel> adapter = new ArrayAdapter(this, android.R.layout.simple_dropdown_item_1line, groupsArray) { | ||
@NonNull | ||
@Override | ||
public View getView(int position, @Nullable View convertView, @NonNull ViewGroup parent) { | ||
View view = super.getView(position, convertView, parent); | ||
TextView textView = view.findViewById(android.R.id.text1); | ||
textView.setText(getItem(position).toString()); | ||
return view; | ||
} | ||
}; | ||
|
||
groupsSelection.setAdapter(adapter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The padding of the dropdown items looks a little off. We have some existing utilities to help with this:
List<VaultGroupModel> groupModels = new ArrayList<>();
groupModels.add(new VaultGroupModel(getString(R.string.new_group)));
groupModels.addAll(groups.stream().map(VaultGroupModel::new).collect(Collectors.toList()));
DropdownHelper.fillDropdown(this, groupsSelection, groupModels);
.setNegativeButton(android.R.string.cancel, null) | ||
.create(); | ||
|
||
dialog.setOnShowListener(d -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this trick to find the positive button or can we pass the listener to setPositiveButton
in this case?
f15cff6
to
797b0fd
Compare
797b0fd
to
4a9f189
Compare
Since we've heavily improved our group management it would only make sense to also improve the flow in which our users assign groups to their entries. This PR adds the ability to tap 'Assign group' whenever they have entries selected.
Fixes #597
Screen.Recording.2024-09-19.001835.mp4