BWA-124 - 'Copy' Option Missing from Long-Press Menu #300

phil-livefront · 2024-12-11T06:12:28Z

🎟️ Tracking

BWA-124

📔 Objective

Implemented the option to copy an item from the long-press menu. While items could already be copied by clicking on them, adding this functionality to the menu was straightforward.
Renamed "Copy to Bitwarden" to "Move to Bitwarden" to align with the updated Figma design.
Refactored the action handling logic to use an enum for dropdown menu actions, consolidating logic and improving maintainability. Added missing tests to cover the new functionality.

📸 Screenshots

Screen_recording_20241211_001056.mp4

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

github-actions · 2024-12-11T06:14:57Z

Checkmarx One – Scan Summary & Details – d035b246-918b-429e-b0f6-92e90265c438

No New Or Fixed Issues Found

...lin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModel.kt

…ress-menu

...va/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingScreenTest.kt

...com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModelTest.kt

dseverns-livefront · 2024-12-11T19:09:52Z

...lin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModel.kt

-
-            is ItemListingAction.EditItemClick -> {
-                handleEditItemClick(action)
+                handleItemClick(action.authCode)


Not a blocker for me, but more a discussion. @david-livefront on the password manager I know we have been avoiding unwrapping actions at this level and always passing the full action into the handle*() function. I am seeing there are somethings in the when that already kind of break away from some of those conventions like sending events, but curious if we want to keep that pattern up in Auth too.

That is a good point, it is a little odd here that these are not actions though. I wonder if the handle moniker is the problem 🤔

dseverns-livefront · 2024-12-11T19:10:54Z

...com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModelTest.kt

+        )
+
+        viewModel.trySendAction(
+            ItemListingAction.DropdownMenuClick(VaultDropdownMenuAction.DELETE, LOCAL_CODE),


⛏️ named args

dseverns-livefront · 2024-12-11T19:11:08Z

...com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModelTest.kt

+
+        viewModel.eventFlow.test {
+            viewModel.trySendAction(
+                ItemListingAction.DropdownMenuClick(VaultDropdownMenuAction.COPY, LOCAL_CODE),


⛏️ named args here too :)

dseverns-livefront · 2024-12-11T19:32:17Z

...lin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModel.kt

                .first { it.data != null }

            val didLaunchAddTotpFlow = authenticatorBridgeManager.startAddTotpLoginItemFlow(
-                totpUri = item.data!!.toOtpAuthUriString(),
+                totpUri = item.data?.toOtpAuthUriString() ?: "",


we can replace the ?: "" with .orEmpty()

david-livefront · 2024-12-11T21:08:57Z

...lin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModel.kt

-                message = R.string.value_has_been_copied.asText(action.authCode),
-            ),
-        )
+    private fun handleItemClick(authCode: String) {


Can we add copy to this function name

yeah good call

SaintPatrck · 2024-12-11T22:11:10Z

...lin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModel.kt

     */
-    data class MoveToBitwardenClick(val entityId: String) : ItemListingAction()
+    data class DropdownMenuClick(


dseverns-livefront

codecov · 2024-12-12T05:29:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (ea1a4e4) to head (3bf48d3).
Report is 1 commits behind head on main.

Additional details and impacted files

@@     Coverage Diff     @@
##   main   #300   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

phil-livefront added 2 commits December 11, 2024 01:00

BWA-124 - 'Copy' Option Missing from Long-Press Menu

74b3d98

add logic to close dropdown menu after copying the code

8470723

phil-livefront requested review from brian-livefront, ahaisting-livefront and a team as code owners December 11, 2024 06:12

phil-livefront added 2 commits December 11, 2024 01:36

fix tests

12ff825

fix linter error

639842d

phil-livefront requested review from SaintPatrck and david-livefront December 11, 2024 15:08

david-livefront reviewed Dec 11, 2024

View reviewed changes

...lin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModel.kt Outdated Show resolved Hide resolved

david-livefront reviewed Dec 11, 2024

View reviewed changes

...lin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModel.kt Outdated Show resolved Hide resolved

phil-livefront added 3 commits December 11, 2024 11:42

PR comments

625cc6c

fix trailing commas

73324ad

Merge branch 'main' into phil/BWA-124-copy-option-missing-from-long-p…

f530626

…ress-menu

david-livefront reviewed Dec 11, 2024

View reviewed changes

...va/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingScreenTest.kt Outdated Show resolved Hide resolved

david-livefront reviewed Dec 11, 2024

View reviewed changes

...com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModelTest.kt Outdated Show resolved Hide resolved

david-livefront reviewed Dec 11, 2024

View reviewed changes

...com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModelTest.kt Outdated Show resolved Hide resolved

PR comments

f184cba

david-livefront previously approved these changes Dec 11, 2024

View reviewed changes

dseverns-livefront reviewed Dec 11, 2024

View reviewed changes

PR comments

7f70f87

phil-livefront dismissed david-livefront’s stale review via 7f70f87 December 11, 2024 20:27

PR comments

bf54b13

david-livefront reviewed Dec 11, 2024

View reviewed changes

PR comments

3bf48d3

SaintPatrck approved these changes Dec 11, 2024

View reviewed changes

dseverns-livefront approved these changes Dec 11, 2024

View reviewed changes

phil-livefront merged commit ad90785 into main Dec 12, 2024
7 checks passed

phil-livefront deleted the phil/BWA-124-copy-option-missing-from-long-press-menu branch December 12, 2024 05:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BWA-124 - 'Copy' Option Missing from Long-Press Menu #300

BWA-124 - 'Copy' Option Missing from Long-Press Menu #300

phil-livefront commented Dec 11, 2024 •

edited by jira bot

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading

dseverns-livefront Dec 11, 2024

david-livefront Dec 11, 2024

dseverns-livefront Dec 11, 2024

dseverns-livefront Dec 11, 2024

dseverns-livefront Dec 11, 2024

david-livefront Dec 11, 2024

phil-livefront Dec 11, 2024

SaintPatrck Dec 11, 2024

dseverns-livefront left a comment

codecov bot commented Dec 12, 2024

BWA-124 - 'Copy' Option Missing from Long-Press Menu #300

BWA-124 - 'Copy' Option Missing from Long-Press Menu #300

Conversation

phil-livefront commented Dec 11, 2024 • edited by jira bot Loading

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

github-actions bot commented Dec 11, 2024 • edited Loading

No New Or Fixed Issues Found

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dseverns-livefront left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 12, 2024

Codecov Report

phil-livefront commented Dec 11, 2024 •

edited by jira bot

Loading

github-actions bot commented Dec 11, 2024 •

edited

Loading