better input validation for bookmarks dialog#4343
Conversation
VishnuSanal
added
the
PR-Awaiting-Initial-Review
|
@VishnuSanal fix CI |
|
codacy cries about using a label in return. seems unreasonable. |
|
|
||
| val result = isValidBookmark(editText1.text.toString(), editText2.text.toString()) | ||
| if (!result.first) { | ||
| Toast.makeText( |
There was a problem hiding this comment.
this should be a snackbar per Android guidelines https://developer.android.com/guide/topics/ui/notifiers/toasts
There was a problem hiding this comment.
I feel like a toast is better in this scenario since snackbar appears below the keyboard. WDYT?
(ideally we should surface this to the dialog box itself like the create dialog - which I guess can be tracked later)
| int i = 0; | ||
| for (String[] x : b) { | ||
| if (x[0].equals(a[0]) && x[1].equals(a[1])) return i; | ||
| if (x[0].equals(a[0]) || x[1].equals(a[1])) return i; |
There was a problem hiding this comment.
Okay, this (DataUtils::constains) function is very opaque, what is it actually doing? Whats the fix doing?
There was a problem hiding this comment.
the changes were because both the name / path should be unique since we cannot have two items with the same name
I have added a javadoc too, thanks!
(but this prevents two bookmarks / servers pointing to the same path with different names - whihc I feel is not a practical usecase)
I think we either need to ignore the issue project wise or ignore on the line to actually be able to merge. |
VishnuSanal
added
PR-Requested-Changes
VishnuSanal
force-pushed
the
bookmark-dialog-fix
branch
from
4bdd551 to
adebdbc
Compare
2 participants
fixes #4337, #4336, #4335 & #4334