Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(web-client): upgrade to angular 13 #146

Merged
merged 24 commits into from
Mar 11, 2022
Merged

build(web-client): upgrade to angular 13 #146

merged 24 commits into from
Mar 11, 2022

Conversation

jongbonga
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Jan 6, 2022

✔️ Deploy Preview for affectionate-mirzakhani-7a7e75 ready!

🔨 Explore the source changes: 9f275b7

🔍 Inspect the deploy log: https://app.netlify.com/sites/affectionate-mirzakhani-7a7e75/deploys/622affd2fd3a5600081aea48

😎 Browse the preview: https://deploy-preview-146--affectionate-mirzakhani-7a7e75.netlify.app

@netlify
Copy link

netlify bot commented Jan 6, 2022

✔️ Deploy Preview for nautilus-wallet ready!

🔨 Explore the source changes: 9f275b7

🔍 Inspect the deploy log: https://app.netlify.com/sites/nautilus-wallet/deploys/622affd20eac3b0008158193

😎 Browse the preview: https://deploy-preview-146--nautilus-wallet.netlify.app

@codecov-commenter
Copy link

Codecov Report

Merging #146 (1f12987) into main (e001adf) will decrease coverage by 3.74%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #146      +/-   ##
==========================================
- Coverage   66.66%   62.91%   -3.75%     
==========================================
  Files          80       77       -3     
  Lines         801      747      -54     
  Branches       85       92       +7     
==========================================
- Hits          534      470      -64     
- Misses        267      277      +10     
Impacted Files Coverage Δ
web-client/src/app/app.module.ts 100.00% <ø> (ø)
...client/src/app/services/enclave/enclave.service.ts 66.66% <100.00%> (-0.84%) ⬇️
web-client/src/app/open-wallet.guard.ts 75.00% <0.00%> (-10.72%) ⬇️
...views/kyc/onfido-widget/onfido-widget.component.ts 72.22% <0.00%> (-4.25%) ⬇️
.../src/app/views/wallet-access/wallet-access.page.ts 48.48% <0.00%> (-3.13%) ⬇️
...client/src/app/views/lockscreen/lockscreen.page.ts 40.00% <0.00%> (-2.11%) ⬇️
web-client/src/app/services/onfido.service.ts 10.34% <0.00%> (-0.77%) ⬇️
web-client/src/app/services/algosdk.utils.ts 12.50% <0.00%> (-0.66%) ⬇️
...b-client/src/app/services/wallet/wallet.service.ts 7.50% <0.00%> (-0.40%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e001adf...1f12987. Read the comment docs.

@jongbonga jongbonga marked this pull request as ready for review March 3, 2022 13:52
Copy link
Contributor

@billguo99 billguo99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running npm install:

57 vulnerabilities (2 low, 20 moderate, 34 high, 1 critical)

Can run npm audit fix can fix a couple of vulnerabilities without any breaking changes:

46 vulnerabilities (2 low, 15 moderate, 29 high)

web-client/tsconfig.json Show resolved Hide resolved
billguo99
billguo99 previously approved these changes Mar 10, 2022
@billguo99 billguo99 dismissed their stale review March 10, 2022 15:35

Waiting for Github Action checks to complete

Copy link
Contributor

@billguo99 billguo99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ScannerPage test works locally, but fails intermittently in Github actions. As discussed in retro, we can approve and merge.

Copy link
Collaborator

@longtomjr longtomjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks a lot for getting this sorted.

@jongbonga jongbonga changed the title Ng13 build(web-client): upgrade to angular 13 Mar 11, 2022
@longtomjr longtomjr merged commit 20e0a54 into main Mar 11, 2022
@longtomjr longtomjr deleted the ng13 branch March 11, 2022 11:57
Copy link
Collaborator

@PiDelport PiDelport left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upgrade seems have broken the use of templateUrl in the theme story: I'm not sure why, yet, but using an inline template instead seems to work, so I did that as a workaround for the time being.

PiDelport added a commit that referenced this pull request Mar 14, 2022
Follows: build(web-client): upgrade to angular 13 #146

This makes the CI pass again.

* test(web-client): temporarily disable failing tests on CI

* chore(web-client): .prettierignore: add Angular cache directory

* style(web-client): npm run format

* refactor(web-client): move lastValueFrom call

This preserves the meaning of the "arrayBuffer" variable.

* test(web-client): use inline template instead of templateUrl

Upgrading to Angular 13 seems to have broken using templateUrl,
but using an inline template still works, as a workaround for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants