[PM-13029] Add copy button items to more fields #1063

aj-rosado · 2024-10-21T14:51:02Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-13029

📔 Objective

Added copy button to itemview Identity and note fields

📸 Screenshots

⏰ 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

… copy button

github-actions · 2024-10-21T15:10:44Z

Checkmarx One – Scan Summary & Details – bf4bc5bf-bd8b-4db4-81f0-92984cda8e75

No New Or Fixed Issues Found

fedemkr · 2024-10-21T15:58:44Z

BitwardenShared/UI/Platform/Application/Views/AccessoryButton.swift

+         action: @escaping () -> Void,
+         accessibilityIdentifier: String = "") {


🎨 Invert the order of this so the action can be called using trailing closures.

fedemkr · 2024-10-21T15:58:56Z

BitwardenShared/UI/Platform/Application/Views/AccessoryButton.swift

+         action: @escaping () async -> Void,
+         accessibilityIdentifier: String = "") {


🎨 Invert the order of this so the action can be called using trailing closures.

fedemkr · 2024-10-21T15:59:08Z

BitwardenShared/UI/Platform/Application/Views/BitwardenTextValueField.swift

+        copyButtonAction: @escaping () -> Void,
+        copyButtonAccessibilityIdentifier: String


🎨 Invert the order of this so the copyButtonAction can be called using trailing closures, if needed.

fedemkr · 2024-10-21T16:01:59Z

BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewIdentityItem/ViewIdentityItemView.swift

-                valueAccessibilityIdentifier: "IdentitySsnEntry"
+                value: socialSecurityNumber,
+                valueAccessibilityIdentifier: "IdentitySsnEntry",
+                copyButtonAction: { store.send(.copyPressed(value: socialSecurityNumber, field: .socialSecurityNumber)) },


⛏️ Line length > 120.

fedemkr · 2024-10-21T16:03:52Z

BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewIdentityItem/ViewIdentityItemView.swift

+                value: socialSecurityNumber,
+                valueAccessibilityIdentifier: "IdentitySsnEntry",
+                copyButtonAction: { store.send(.copyPressed(value: socialSecurityNumber, field: .socialSecurityNumber)) },
+                copyButtonAccessibilityIdentifier: "IdentityCopySocialSecurityNumberButton"


🎨 As the value accessibility identifier we're using IdentitySsnEntry with "Social security number" as "Ssn", so unless QA automation folks think differently we should maintain the same approach here as well => "IdentityCopySsnButton".

fedemkr · 2024-10-21T16:05:08Z

BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewIdentityItem/ViewIdentityItemView.swift

-                title: Localizations.email,
+                title: email,


⚠️ I believe the title should be the localized string email. Was there a product change for this?

fedemkr · 2024-10-21T16:05:59Z

BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewItemAction.swift

+    /// The identity address field.
+    case notes


⛏️ It seems the comment is wrong.

codecov · 2024-10-22T14:04:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.39%. Comparing base (cee1ad4) to head (ac1823b).
Report is 15 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
+ Coverage   89.37%   89.39%   +0.02%     
==========================================
  Files         672      672              
  Lines       42364    42455      +91     
==========================================
+ Hits        37861    37954      +93     
+ Misses       4503     4501       -2

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

# Conflicts: # BitwardenShared/UI/Vault/VaultItem/ViewItem/ViewItemDetailsView.swift # BitwardenShared/UI/Vault/VaultItem/ViewItem/__Snapshots__/ViewItemViewTests/test_snapshot_identity_withAllValues_largeText.1.png # BitwardenShared/UI/Vault/VaultItem/ViewItem/__Snapshots__/ViewItemViewTests/test_snapshot_login_disabledViewPassword.1.png # BitwardenShared/UI/Vault/VaultItem/ViewItem/__Snapshots__/ViewItemViewTests/test_snapshot_login_hiddenTotp.1.png # BitwardenShared/UI/Vault/VaultItem/ViewItem/__Snapshots__/ViewItemViewTests/test_snapshot_login_withAllValues.1.png # BitwardenShared/UI/Vault/VaultItem/ViewItem/__Snapshots__/ViewItemViewTests/test_snapshot_login_withAllValues_exceptTotp_noPremium.1.png # BitwardenShared/UI/Vault/VaultItem/ViewItem/__Snapshots__/ViewItemViewTests/test_snapshot_login_withAllValues_noPremium.1.png # BitwardenShared/UI/Vault/VaultItem/ViewItem/__Snapshots__/ViewItemViewTests/test_snapshot_previews_login.1.png # BitwardenShared/UI/Vault/VaultItem/ViewItem/__Snapshots__/ViewItemViewTests/test_snapshot_previews_login_dark.1.png

fedemkr

⛏️ Check automatic PR files annotations to fix warnings and also add missing tests.

fedemkr

Looks good 🎉

aj-rosado added 2 commits October 21, 2024 16:25

Added init to BitwardenTextValueField that accepts values to create a…

0c397af

… copy button

added copy button to itemview fields missing it

b1eb2d6

fedemkr changed the title ~~[PM-13029] add copy button items to more fields~~ [PM-13029] Add copy button items to more fields Oct 21, 2024

fedemkr reviewed Oct 21, 2024

View reviewed changes

aj-rosado added 3 commits October 22, 2024 11:49

Addressed PR comments

8c24a93

Merge branch 'main' into PM-13029/add-copy-button-items

a4bd076

updated snapshots

92098a3

aj-rosado added 3 commits October 23, 2024 11:49

Added unit tests for the new copy buttons

cb4edd6

updated snapshots with new icons

d06e2ca

aj-rosado marked this pull request as ready for review October 23, 2024 12:06

aj-rosado requested review from matt-livefront, phil-livefront, victor-livefront, brant-livefront and a team as code owners October 23, 2024 12:06

aj-rosado requested a review from fedemkr October 23, 2024 12:06

fedemkr reviewed Oct 24, 2024

View reviewed changes

Added more tests and fixed identation

ac1823b

aj-rosado force-pushed the PM-13029/add-copy-button-items branch from 0222ed1 to ac1823b Compare October 25, 2024 15:27

aj-rosado requested a review from fedemkr October 25, 2024 15:35

fedemkr approved these changes Oct 25, 2024

View reviewed changes

aj-rosado merged commit ca586ba into main Oct 28, 2024
9 checks passed

aj-rosado deleted the PM-13029/add-copy-button-items branch October 28, 2024 11:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-13029] Add copy button items to more fields #1063

[PM-13029] Add copy button items to more fields #1063

aj-rosado commented Oct 21, 2024

github-actions bot commented Oct 21, 2024 •

edited

Loading

fedemkr Oct 21, 2024

fedemkr Oct 21, 2024

fedemkr Oct 21, 2024

fedemkr Oct 21, 2024

fedemkr Oct 21, 2024

fedemkr Oct 21, 2024

fedemkr Oct 21, 2024

codecov bot commented Oct 22, 2024 •

edited

Loading

fedemkr left a comment

fedemkr left a comment

		action: @escaping () -> Void,
		accessibilityIdentifier: String = "") {

		action: @escaping () async -> Void,
		accessibilityIdentifier: String = "") {

		copyButtonAction: @escaping () -> Void,
		copyButtonAccessibilityIdentifier: String

[PM-13029] Add copy button items to more fields #1063

[PM-13029] Add copy button items to more fields #1063

Conversation

aj-rosado commented Oct 21, 2024

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

github-actions bot commented Oct 21, 2024 • edited Loading

No New Or Fixed Issues Found

fedemkr Oct 21, 2024

Choose a reason for hiding this comment

fedemkr Oct 21, 2024

Choose a reason for hiding this comment

fedemkr Oct 21, 2024

Choose a reason for hiding this comment

fedemkr Oct 21, 2024

Choose a reason for hiding this comment

fedemkr Oct 21, 2024

Choose a reason for hiding this comment

fedemkr Oct 21, 2024

Choose a reason for hiding this comment

fedemkr Oct 21, 2024

Choose a reason for hiding this comment

codecov bot commented Oct 22, 2024 • edited Loading

Codecov Report

fedemkr left a comment

Choose a reason for hiding this comment

fedemkr left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 21, 2024 •

edited

Loading

codecov bot commented Oct 22, 2024 •

edited

Loading