626, 627: Add kebab menu for delete #764

eliserichards · 2019-06-18T21:19:00Z

Fixes #626
Fixes #627
Fixes #629

This PR creates the three-dot kebab menu in the item details with the Delete option. On selecting Delete, the confirmation dialog is displayed. Once confirmed, you are routed back to the item list and a toast notification is displayed with the "hostname deleted".

Testing and Review Notes

Duplicated the spinner logic from the sort menu in the ItemList. Will make this cleaner #todo
~~Not sure about the placement of the enums and Actions. Should they go with the Routing, the ItemDetail stuff, the DataStore stuff....?~~

Screenshots or Videos

To Do - Engineering

To Do - Other

add “WIP” to the PR title if pushing up but not complete nor ready for review
double check the original issue to confirm it is fully satisfied
add testing notes and screenshots in PR description to help guide reviewers
add unit tests
request the "UX" team perform a design review (if/when applicable)
make sure CI builds are passing (e.g.: fix lint and other errors)
check on the accessibility of any added UI

…bar.

…igating back.

linuxwolf

I think it's moving in the right direction.

My biggest concern is not explicitly syncing once a Login is deleted. Next is with the potential abuse of RouteActions, maybe?

Don't forget to update the tests, especially with the new properties/methods added to the ItemDetailView interface!

app/src/main/java/mozilla/lockbox/view/DialogFragment.kt

app/src/main/java/mozilla/lockbox/action/RouteAction.kt

app/src/main/java/mozilla/lockbox/presenter/ItemDetailPresenter.kt

app/src/main/java/mozilla/lockbox/store/DataStore.kt

app/src/main/java/mozilla/lockbox/view/ItemDetailFragment.kt

Addressed comments :)

linuxwolf

Codewise looks even better!

In testing, I've found a couple of concerns:

Every toast on the ItemList seems to pop up with " deleted"
Tapping the System Back from the ListItem takes the user back to the "Delete item?", which if canceled still reveals the deleted login.

The System Back is a more systemic issue that isn't valid to try to fix here.

app/src/main/java/mozilla/lockbox/store/DataStore.kt

linuxwolf · 2019-07-02T21:09:43Z

app/src/main/java/mozilla/lockbox/view/ItemListFragment.kt

-        toast.show()
+
+        val view = toast.view.findViewById(R.id.message) as TextView
+        view.text = (text ?: resources.getString(strId!!)).plus(" deleted.")


this ends up showing " deleted" at the end of all toast notifications. I don't think that's the intent.

This seems to be some sort of caching with the toasts, because this method is only called for the delete flow and not "sync timeout" etc.

It looks like setUpToast is also called from showToastNotification, which is in turned called in ItemListPresenter.

Orrrrr I accidentally switched the elvis values. Fixed now.

Addressed toast comments. Backable issues will be fixed in separate PR.

eliserichards · 2019-07-02T23:19:37Z

Got the g2g from @linuxwolf, going to merge and followup with @changecourse on additional PRs.

eliserichards · 2019-07-02T23:29:45Z

Backable issue filed: #804

linuxwolf

obligatoryfor realz r+

This reverts commit 17605c3.

* Revert "828-l10nScreenshotsTest-delete (#829)" This reverts commit bab9fe8. * Revert "806-uitest-add-delete (#808)" This reverts commit ec56009. * Revert "626, 627: Add kebab menu for delete (#764)" This reverts commit 17605c3. * Remove unused imports and fix lint error. * Rebase and lint

* Add release notes for 1.1.2.4405 * Bump version to 1.2.0 * Add ignore to the flaky test. This needs to be fixed later. (#793) * 626, 627: Add kebab menu for delete (#764) * Add kebab menu icon and strings. Set up fragment for addition to toolbar. * Create fragment and dialog. * Add confirmation dialog, actions, and presenters * Routing * Ensure item is actually deleted and item list is refreshed before navigating back. * Refactor and remove edit for now * Keeping edit button, just not doing any functionality with it * Test for item detail presenter * Ellipsize long item titles in detail view * Delete confirmation toast * Delete toast and tests * Consumable toast notification for delete * Specify return type on Optional extension * ItemListPresenterTest needed a consumable for the delete toast test * Cleanup * Fix toast notifications * Import strings from android-l10n (#809) State: mozilla-l10n/android-l10n@63a5240 * 806-uitest-add-delete (#808) * Adjust spinner dropdown size to fit longer words (l10n) (#812) * Ellipsize text in item list (#811) * 810: Reset support when syncing (#826) * Reset support when info is null * Reset support every time we sync. * Removing timeout to be fixed in #791 (#833) * Removing timeout to be fixed in #791 * Remove strings * 828-l10nScreenshotsTest-delete (#829) * Add dot to sentence (#837) * 814: Telemetry for sync (#835) * Add telemetry actions for sync, list update, and error states. * Sync timeout instead of sync end * Add sync complete event * Add errors and ids to telemetry calls * Update metrics docs to reflect telemetry sync changes (#841) * Revert delete PRs (#836) * Revert "828-l10nScreenshotsTest-delete (#829)" This reverts commit bab9fe8. * Revert "806-uitest-add-delete (#808)" This reverts commit ec56009. * Revert "626, 627: Add kebab menu for delete (#764)" This reverts commit 17605c3. * Remove unused imports and fix lint error. * Rebase and lint * Update to version 1.1.3 (#844) * Update version to 1.1.3 * Add release notes for 1.1.3

* Add release notes for 1.1.2.4405 * Bump version to 1.2.0 * Add ignore to the flaky test. This needs to be fixed later. (#793) * 626, 627: Add kebab menu for delete (#764) * Add kebab menu icon and strings. Set up fragment for addition to toolbar. * Create fragment and dialog. * Add confirmation dialog, actions, and presenters * Routing * Ensure item is actually deleted and item list is refreshed before navigating back. * Refactor and remove edit for now * Keeping edit button, just not doing any functionality with it * Test for item detail presenter * Ellipsize long item titles in detail view * Delete confirmation toast * Delete toast and tests * Consumable toast notification for delete * Specify return type on Optional extension * ItemListPresenterTest needed a consumable for the delete toast test * Cleanup * Fix toast notifications * Import strings from android-l10n (#809) State: mozilla-l10n/android-l10n@63a5240 * 806-uitest-add-delete (#808) * Adjust spinner dropdown size to fit longer words (l10n) (#812) * Ellipsize text in item list (#811) * 810: Reset support when syncing (#826) * Reset support when info is null * Reset support every time we sync. * Removing timeout to be fixed in #791 (#833) * Removing timeout to be fixed in #791 * Remove strings * 828-l10nScreenshotsTest-delete (#829) * Add dot to sentence (#837) * 814: Telemetry for sync (#835) * Add telemetry actions for sync, list update, and error states. * Sync timeout instead of sync end * Add sync complete event * Add errors and ids to telemetry calls * Update metrics docs to reflect telemetry sync changes (#841) * Revert delete PRs (#836) * Revert "828-l10nScreenshotsTest-delete (#829)" This reverts commit bab9fe8. * Revert "806-uitest-add-delete (#808)" This reverts commit ec56009. * Revert "626, 627: Add kebab menu for delete (#764)" This reverts commit 17605c3. * Remove unused imports and fix lint error. * Rebase and lint * Update to version 1.1.3 (#844) * Update version to 1.1.3 * Add release notes for 1.1.3 * Update releases.md (#847) * Add more context to release notes * Add descriptions for proguard rules upload. * Revert "Revert delete PRs (#836)" (#842) This reverts commit 974a585. * 816: string updates for localization (#852) * Revert "Revert delete PRs (#836)" This reverts commit 974a585. * Lockwise placeholder in delete_description and descriptive comment in edit string * Update app services, android components, and megazord configuration (#865) * Update app services, android components, and megazord configuration * Update a-c to v7.0.0 * Send hashed UID ping when sync completes successfully. * Remove handling for synctelemetryping, to be completed in separate issue. Update AS to 0.37.1 * Update Readme with l10n process (#872) This is something we've been adding to all Readmes of mobile repos to direct people to Pontoon for localization (vs GitHub repos, where work isn't supposed to happen). * 867 - Fix obvious NPE sync crash (#884) * 871: Create infrastructure for feature flags (#882) * Delete feature flag * Move feature flags to separate support file * Create support to check build configs. Create flags for CUD * Add telemetry probes to syncIfRequired trigger syncs (#887) * Import strings from l10n (#893) * Import strings from android-l10n State: mozilla-l10n/android-l10n@349efdb * Commenting unused string for Espanol * 791: Remove fake timeout from sync (#889) * Remove fake timeout from sync * Remove all sync timeout strings * Rebase and lint * Fail fast if the edge between to routes is not in the nav graph. (#895) * Auto-signin with mozilla AuthProvider (#879) — behind a feature flag. * WIP Automatic FxA sign-in support * Cleanup welcome fragment/account detection * Upgrade to Android Components 9.0.0 * Ensure landable, even if not releasable * gradlew lint * 628: edit view and routing (#843) * Edit xml layout * Add route, presenter, and fragment Button clicks and dialog Update dependencies Edit presenter tests Routing stuff Usable state with spinner Buttons working Password visibility in edit mode Lint View Create popup item and insert menu into detail view's kebab buttton * Popup menu and click listener Dropdown menu and formatting Update list - in progress DataStoreTest - mocked up test, stuck on init Datastore update item detail test * Save edited changes Lint Update dependencies Edit unit tests Add sync to datastore editing. Update var name in robot and screenshot tests Reformatting dropdown menu and edit view Adding inclusive popup to edit->detail view nav definition Address nullable server passwords for delete and edit. Remove unused string. Remove learn more clicks from test Refactoring pushError into a helper method. * Add fxalogin to autofill onboarding route. (#901) * 798: routing back to itemlist on login/search/feedback forms (#890) * Investigation * Add locking routes. * Explicitly ignore autofill on search and edit fields * Ignore autofill on webviews * Remove comments and clean up lint * Add null text value for empty usernames (#902) * Add null text value for empty usernames * gradlew lint * 907 fix ui tests (#908) * 907: fix testSettings * fixing routing issues * fixing more routes * changing button name and adding route * fixing conflicts after rebase * Update release-notes.md with v1.1.4 * Fix lint errors * Update release-notes.md to correct bitrise version

* Add release notes for 1.1.2.4405 * Bump version to 1.2.0 * Add ignore to the flaky test. This needs to be fixed later. (#793) * 626, 627: Add kebab menu for delete (#764) * Add kebab menu icon and strings. Set up fragment for addition to toolbar. * Create fragment and dialog. * Add confirmation dialog, actions, and presenters * Routing * Ensure item is actually deleted and item list is refreshed before navigating back. * Refactor and remove edit for now * Keeping edit button, just not doing any functionality with it * Test for item detail presenter * Ellipsize long item titles in detail view * Delete confirmation toast * Delete toast and tests * Consumable toast notification for delete * Specify return type on Optional extension * ItemListPresenterTest needed a consumable for the delete toast test * Cleanup * Fix toast notifications * Import strings from android-l10n (#809) State: mozilla-l10n/android-l10n@63a5240 * 806-uitest-add-delete (#808) * Adjust spinner dropdown size to fit longer words (l10n) (#812) * Ellipsize text in item list (#811) * 810: Reset support when syncing (#826) * Reset support when info is null * Reset support every time we sync. * Removing timeout to be fixed in #791 (#833) * Removing timeout to be fixed in #791 * Remove strings * 828-l10nScreenshotsTest-delete (#829) * Add dot to sentence (#837) * 814: Telemetry for sync (#835) * Add telemetry actions for sync, list update, and error states. * Sync timeout instead of sync end * Add sync complete event * Add errors and ids to telemetry calls * Update metrics docs to reflect telemetry sync changes (#841) * Revert delete PRs (#836) * Revert "828-l10nScreenshotsTest-delete (#829)" This reverts commit bab9fe8. * Revert "806-uitest-add-delete (#808)" This reverts commit ec56009. * Revert "626, 627: Add kebab menu for delete (#764)" This reverts commit 17605c3. * Remove unused imports and fix lint error. * Rebase and lint * Update to version 1.1.3 (#844) * Update version to 1.1.3 * Add release notes for 1.1.3 * Update releases.md (#847) * Add more context to release notes * Add descriptions for proguard rules upload. * Revert "Revert delete PRs (#836)" (#842) This reverts commit 974a585. * 816: string updates for localization (#852) * Revert "Revert delete PRs (#836)" This reverts commit 974a585. * Lockwise placeholder in delete_description and descriptive comment in edit string * Update app services, android components, and megazord configuration (#865) * Update app services, android components, and megazord configuration * Update a-c to v7.0.0 * Send hashed UID ping when sync completes successfully. * Remove handling for synctelemetryping, to be completed in separate issue. Update AS to 0.37.1 * Update Readme with l10n process (#872) This is something we've been adding to all Readmes of mobile repos to direct people to Pontoon for localization (vs GitHub repos, where work isn't supposed to happen). * 867 - Fix obvious NPE sync crash (#884) * 871: Create infrastructure for feature flags (#882) * Delete feature flag * Move feature flags to separate support file * Create support to check build configs. Create flags for CUD * Add telemetry probes to syncIfRequired trigger syncs (#887) * Import strings from l10n (#893) * Import strings from android-l10n State: mozilla-l10n/android-l10n@349efdb * Commenting unused string for Espanol * 791: Remove fake timeout from sync (#889) * Remove fake timeout from sync * Remove all sync timeout strings * Rebase and lint * Fail fast if the edge between to routes is not in the nav graph. (#895) * Auto-signin with mozilla AuthProvider (#879) — behind a feature flag. * WIP Automatic FxA sign-in support * Cleanup welcome fragment/account detection * Upgrade to Android Components 9.0.0 * Ensure landable, even if not releasable * gradlew lint * 628: edit view and routing (#843) * Edit xml layout * Add route, presenter, and fragment Button clicks and dialog Update dependencies Edit presenter tests Routing stuff Usable state with spinner Buttons working Password visibility in edit mode Lint View Create popup item and insert menu into detail view's kebab buttton * Popup menu and click listener Dropdown menu and formatting Update list - in progress DataStoreTest - mocked up test, stuck on init Datastore update item detail test * Save edited changes Lint Update dependencies Edit unit tests Add sync to datastore editing. Update var name in robot and screenshot tests Reformatting dropdown menu and edit view Adding inclusive popup to edit->detail view nav definition Address nullable server passwords for delete and edit. Remove unused string. Remove learn more clicks from test Refactoring pushError into a helper method. * Add fxalogin to autofill onboarding route. (#901) * 798: routing back to itemlist on login/search/feedback forms (#890) * Investigation * Add locking routes. * Explicitly ignore autofill on search and edit fields * Ignore autofill on webviews * Remove comments and clean up lint * Add null text value for empty usernames (#902) * Add null text value for empty usernames * gradlew lint * 907 fix ui tests (#908) * 907: fix testSettings * fixing routing issues * fixing more routes * changing button name and adding route * fixing conflicts after rebase * Update release-notes.md with v1.1.4 * Update release notes for v2.0.0 * Update version code and version name to v2.0.0

eliserichards force-pushed the 626-delete-kebab branch from 7a5c929 to 0f33e9d Compare June 25, 2019 22:17

eliserichards marked this pull request as ready for review June 25, 2019 23:51

eliserichards requested a review from a team as a code owner June 25, 2019 23:51

eliserichards force-pushed the 626-delete-kebab branch from 1d392ed to 2404d2b Compare June 27, 2019 01:36

eliserichards requested review from changecourse and linuxwolf June 27, 2019 01:38

Elise Richards added 5 commits June 27, 2019 14:07

Add kebab menu icon and strings. Set up fragment for addition to tool…

88cd8fb

…bar.

Create fragment and dialog.

f3809b7

Add confirmation dialog, actions, and presenters

874161b

Routing

9e551c9

Ensure item is actually deleted and item list is refreshed before nav…

8159ac0

…igating back.

linuxwolf previously requested changes Jun 28, 2019

View reviewed changes

Elise Richards added 3 commits July 1, 2019 13:46

Refactor and remove edit for now

974dff9

Keeping edit button, just not doing any functionality with it

be371a3

Test for item detail presenter

b80a52d

eliserichards force-pushed the 626-delete-kebab branch from 2404d2b to b80a52d Compare July 1, 2019 21:20

eliserichards requested a review from linuxwolf July 1, 2019 21:22

Ellipsize long item titles in detail view

5c23cde

eliserichards changed the title ~~[WIP] 626: Add kebab menu for delete~~ 626, 627: Add kebab menu for delete Jul 1, 2019

Elise Richards added 6 commits July 1, 2019 15:30

Delete confirmation toast

ac2fc24

Delete toast and tests

fead6a9

Consumable toast notification for delete

3a89346

Specify return type on Optional extension

312f2d2

ItemListPresenterTest needed a consumable for the delete toast test

57759e2

Cleanup

de106b2

linuxwolf previously requested changes Jul 2, 2019

View reviewed changes

Fix toast notifications

1df8fa6

linuxwolf approved these changes Jul 2, 2019

View reviewed changes

eliserichards merged commit 17605c3 into master Jul 3, 2019

eliserichards pushed a commit that referenced this pull request Jul 19, 2019

Revert "626, 627: Add kebab menu for delete (#764)"

c434f12

This reverts commit 17605c3.

eliserichards mentioned this pull request Jul 19, 2019

Revert delete PRs #836

Merged

3 tasks

eliserichards pushed a commit that referenced this pull request Jul 24, 2019

Revert "626, 627: Add kebab menu for delete (#764)"

392765a

This reverts commit 17605c3.

eliserichards mentioned this pull request Jul 24, 2019

Revert "Revert delete PRs (#836)" #842

Merged

3 tasks

eliserichards deleted the 626-delete-kebab branch October 23, 2019 17:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

626, 627: Add kebab menu for delete #764

626, 627: Add kebab menu for delete #764

eliserichards commented Jun 18, 2019 •

edited

Loading

linuxwolf left a comment

linuxwolf left a comment

linuxwolf Jul 2, 2019

eliserichards Jul 2, 2019

linuxwolf Jul 2, 2019 •

edited

Loading

eliserichards Jul 2, 2019

eliserichards commented Jul 2, 2019

eliserichards commented Jul 2, 2019

linuxwolf left a comment

626, 627: Add kebab menu for delete #764

626, 627: Add kebab menu for delete #764

Conversation

eliserichards commented Jun 18, 2019 • edited Loading

Testing and Review Notes

Screenshots or Videos

To Do - Engineering

To Do - Other

linuxwolf left a comment

Choose a reason for hiding this comment

linuxwolf left a comment

Choose a reason for hiding this comment

linuxwolf Jul 2, 2019

Choose a reason for hiding this comment

eliserichards Jul 2, 2019

Choose a reason for hiding this comment

linuxwolf Jul 2, 2019 • edited Loading

Choose a reason for hiding this comment

eliserichards Jul 2, 2019

Choose a reason for hiding this comment

eliserichards commented Jul 2, 2019

eliserichards commented Jul 2, 2019

linuxwolf left a comment

Choose a reason for hiding this comment

eliserichards commented Jun 18, 2019 •

edited

Loading

linuxwolf Jul 2, 2019 •

edited

Loading