[CL-106] use CL's DialogService in Desktop & Browser #5875

willmartian · 2023-07-22T13:42:31Z

Type of change

- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Relies on: #5890

Objective

Desktop and Browser will now use the CL's DialogService
- The DialogServiceAbstraction was deleted, and all imports of it were updated to the CL's DialogService.
- DialogService.open is now available in Desktop and Browser, meaning the ModalService can be fully deprecated.
- The BrowserDialogService and ElectronDialogService were deleted. SimpleDialogs in all clients will now use the CL implementation.
Resolve circular dependency between libs/angular and libs/components
- SimpleDialogComponent and related files were moved to libs/components.
- libs/angular is now a restricted import in the CL
SimpleDialogType and SimpleDialogCloseType are now types instead of enum
- After moving the enums to the CL, I was running into an issue where they were undefined at runtime in test files. This looks like an issue with ts-jest and re-exporting enums in multiple files: [Bug]: Exported enums not working on tests kulshekhar/ts-jest#3397
- (I also dislike TS enums in general, and since I am diffing all of these files anyways, might as well use this as my chance to get rid of the enum. 😈)

Code changes

.eslintrc.json: Added restricted imports
libs/angular/src/services/modal.service.ts: Added deprecation note
libs/components/src/dialog/dialog.service.ts: Updated imports; removed DialogServiceAbstraction; added translate method that is called by SimpleDialogComponent
libs/components/src/dialog/simple-dialog/types.ts: Colocated SimpleDialog types
libs/shared/tsconfig.libs.json: Added @bitwarden/components import alias
libs/angular/src/services/dialog/*: deleted
Everything else: Updated imports to DialogServiceAbstraction; removed SimpleDialog enums

Screenshots

DialogService.open with bit-dialog in browser extension:

Screen.Recording.2023-07-24.at.10.19.33.AM.mov

DialogService.open with bit-dialog on desktop:

Screen.Recording.2023-07-24.at.10.36.56.AM.mov

Before you submit

Please add unit tests where it makes sense to do so (encouraged but not required)
If this change requires a documentation update - notify the documentation team
If this change has particular deployment requirements - notify the DevOps team
Ensure that all UI additions follow WCAG AA requirements

bitwarden-bot · 2023-07-22T14:39:01Z

Checkmarx One – Scan Summary & Details – 7ea08dac-58dd-4f86-8c0a-4b50c01491f2

Fixed Issues

Issue	Source File / Package	Checkmarx Insight
Use_Of_Hardcoded_Password	/libs/importer/src/importers/keepass2-xml-importer.ts: 60	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/passwordsafe-xml-importer.ts: 35	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/passworddragon-xml-importer.ts: 28	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/keepass2-xml-importer.ts: 114	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/keepass2-xml-importer.ts: 67	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/keepass2-xml-importer.ts: 71	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/keepass2-xml-importer.ts: 72	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 132	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/passwordsafe-xml-importer.ts: 35	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/passworddragon-xml-importer.ts: 28	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 56	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 53	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 52	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 51	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 50	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 49	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 48	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 47	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 46	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 45	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 44	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 43	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 42	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 41	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 40	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 39	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 38	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 37	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 36	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 35	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 34	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 33	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 32	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 31	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 30	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 29	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 28	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 27	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 26	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 56	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 53	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 52	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 51	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 50	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 49	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 48	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 47	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 46	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 45	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 44	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 43	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 42	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 41	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 40	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 39	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 38	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 37	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 36	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 35	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 34	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 33	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 32	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 31	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 30	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 29	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 28	Attack Vector
Use_Of_Hardcoded_Password	/libs/importer/src/importers/base-importer.ts: 27	Attack Vector
Use_Of_Hardcoded_Password

More results are available on AST platform

.eslintrc.json

libs/components/src/dialog/simple-dialog/types.ts

apps/desktop/src/app/tools/export/export.component.ts

apps/web/src/app/billing/settings/payment-method.component.ts

libs/components/src/dialog/dialog.service.ts

djsmith85

Changes are looking great! Nice work on this 🚀

shane-melton

Changes related to AC look good!

Thomas-Avery

SM changes look good, thank you.

jlf0dev

Looks good! I feel the same about enums so nice to see the unions. Not importing makes it slightly more difficult to refactor/rename, but seems like everyone else is good with that so 👍

JaredSnider-Bitwarden

LGTM!

Only a single non blocking question.

JaredSnider-Bitwarden · 2023-08-16T00:09:14Z

libs/angular/src/services/modal.service.ts

@@ -21,6 +21,9 @@ export class ModalConfig<D = any> {
  replaceTopModal?: boolean;
 }

+/**
+ * @deprecated Use the Component Library's `DialogService` instead.


Minor question (non-blocking): should we include a target date + tech debt item for cleaning up and removing this service?

gbubemismith

LGTM!

cyprain-okeke

LGTM!

willmartian added 6 commits July 21, 2023 23:21

remove libs/angular dialog service; move simple dialog types to CL

ae048a8

update DialogServiceAbstraction imports to CL

47746a8

update imports in libs/angular to use CL

939ab04

colocate simple dialog types

b67e433

move SimpleConfigurableDialog files under SimpleDialog

0402c95

remove CL import alias from CL src

c391a6d

github-actions bot added the needs-qa Marks a PR as requiring QA approval label Jul 22, 2023

willmartian added 2 commits July 22, 2023 10:28

update imports

c196b61

run prettier

4200341

willmartian added 3 commits July 22, 2023 11:30

convert SimpleDialog enums to types

4d76e41

replace DialogServiceAbstraction with DialogService

d75b22c

restrict libs/angular imports in CL

1c8571d

willmartian changed the title ~~[CL-106] dialog service desktop browser~~ [CL-106] use CL's DialogService in Desktop & Browser Jul 24, 2023

add deprecation note to ModalService

2a41541

willmartian requested review from Hinton and JaredSnider-Bitwarden July 24, 2023 14:39

willmartian mentioned this pull request Jul 26, 2023

[PM-3334] Update desktop dark theme to match browser #5890

Merged

willmartian added 4 commits August 7, 2023 16:20

Delete BrowserDialogService

3b120d2

Remove ElectronDialogService

f64fc22

update browser and desktop services.module

6ea207b

resolve merge conflicts

c127c95

willmartian marked this pull request as ready for review August 8, 2023 03:49

willmartian requested review from a team as code owners August 8, 2023 03:49

willmartian requested a review from a team as a code owner August 8, 2023 03:49

Hinton reviewed Aug 8, 2023

View reviewed changes

.eslintrc.json Show resolved Hide resolved

libs/components/src/dialog/simple-dialog/types.ts Outdated Show resolved Hide resolved

apps/desktop/src/app/tools/export/export.component.ts Outdated Show resolved Hide resolved

cyprain-okeke reviewed Aug 8, 2023

View reviewed changes

apps/web/src/app/billing/settings/payment-method.component.ts Show resolved Hide resolved

willmartian added 3 commits August 8, 2023 11:14

remove os.EOL in simple dialog

dc725c8

change SimpleDialogCloseType to boolean

86b4392

resolve merge conflicts

993c309

willmartian requested a review from Hinton August 8, 2023 16:29

resolve merge conflicts

bed715d

willmartian requested a review from cyprain-okeke August 15, 2023 03:22

Hinton reviewed Aug 15, 2023

View reviewed changes

libs/components/src/dialog/dialog.service.ts Outdated Show resolved Hide resolved

Hinton previously approved these changes Aug 15, 2023

View reviewed changes

djsmith85 previously approved these changes Aug 15, 2023

View reviewed changes

willmartian added 2 commits August 15, 2023 16:28

remove close type

b9db815

resolve merge conflicts

7d6bcba

willmartian dismissed stale reviews from djsmith85 and Hinton via 7d6bcba August 15, 2023 20:46

willmartian requested review from Hinton and djsmith85 August 15, 2023 21:05

djsmith85 approved these changes Aug 15, 2023

View reviewed changes

shane-melton approved these changes Aug 15, 2023

View reviewed changes

Hinton approved these changes Aug 15, 2023

View reviewed changes

Thomas-Avery approved these changes Aug 15, 2023

View reviewed changes

jlf0dev approved these changes Aug 15, 2023

View reviewed changes

JaredSnider-Bitwarden approved these changes Aug 16, 2023

View reviewed changes

gbubemismith approved these changes Aug 16, 2023

View reviewed changes

cyprain-okeke approved these changes Aug 16, 2023

View reviewed changes

willmartian removed the needs-qa Marks a PR as requiring QA approval label Aug 16, 2023

willmartian merged commit a4fcd62 into master Aug 16, 2023
55 of 56 checks passed

willmartian deleted the pm/CL-106-dialog-service-desktop-browser branch August 16, 2023 12:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CL-106] use CL's DialogService in Desktop & Browser #5875

[CL-106] use CL's DialogService in Desktop & Browser #5875

willmartian commented Jul 22, 2023 •

edited

Loading

bitwarden-bot commented Jul 22, 2023 •

edited

Loading

djsmith85 left a comment

shane-melton left a comment

Thomas-Avery left a comment

jlf0dev left a comment

JaredSnider-Bitwarden left a comment

JaredSnider-Bitwarden Aug 16, 2023

gbubemismith left a comment

cyprain-okeke left a comment

[CL-106] use CL's DialogService in Desktop & Browser #5875

[CL-106] use CL's DialogService in Desktop & Browser #5875

Conversation

willmartian commented Jul 22, 2023 • edited Loading

Type of change

Objective

Code changes

Screenshots

Before you submit

bitwarden-bot commented Jul 22, 2023 • edited Loading

Fixed Issues

djsmith85 left a comment

Choose a reason for hiding this comment

shane-melton left a comment

Choose a reason for hiding this comment

Thomas-Avery left a comment

Choose a reason for hiding this comment

jlf0dev left a comment

Choose a reason for hiding this comment

JaredSnider-Bitwarden left a comment

Choose a reason for hiding this comment

JaredSnider-Bitwarden Aug 16, 2023

Choose a reason for hiding this comment

gbubemismith left a comment

Choose a reason for hiding this comment

cyprain-okeke left a comment

Choose a reason for hiding this comment

willmartian commented Jul 22, 2023 •

edited

Loading

bitwarden-bot commented Jul 22, 2023 •

edited

Loading