-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[PM-4347] Upgrade angular to 17 #11031
Conversation
No New Or Fixed Issues Found |
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #11031 +/- ##
==========================================
+ Coverage 33.51% 33.53% +0.02%
==========================================
Files 2815 2815
Lines 87564 87646 +82
Branches 16695 16694 -1
==========================================
+ Hits 29344 29395 +51
- Misses 55911 55942 +31
Partials 2309 2309 ☔ View full report in Codecov by Sentry. |
# Conflicts: # apps/web/src/app/vault/individual-vault/vault.component.ts # package-lock.json # package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, not much to say 🙂
Seems like there's one broken story due to @
being present in the HTML
package.json
Outdated
@@ -27,27 +27,28 @@ | |||
"storybook": "ng run components:storybook", | |||
"build-storybook": "ng run components:build-storybook", | |||
"build-storybook:ci": "ng run components:build-storybook --webpack-stats-json", | |||
"postinstall": "patch-package" | |||
"postinstall": "patch-package && rimraf ./node_modules/@types/glob && rimraf ./node_modules/@types/minimatch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: will this fix: #11542 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Hinton question for you probably.
@@ -71,7 +71,7 @@ The content can be a button, anchor, or static container. | |||
<bit-item> | |||
<button bit-item-content type="button"> | |||
<bit-avatar slot="start" text="Foo"></bit-avatar> | |||
[email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bitwarden/team-design-system Since this is just rendering text it would be fine for us to have @
for readability or also switch it to @
so we aren't showing off invalid HTML, any preference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm I am fine with either, just not really understanding why it broke 🤔 I guess it's a reserved character for Angular html?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's always technically not right to use it but likely angular got much more strict about it with the addition of the new control flow syntax: https://angular.dev/guide/templates/control-flow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay that makes sense. Then yes I agree with not showing invalid html in the docs file!
# Conflicts: # package-lock.json # package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vault change looks good
# Conflicts: # package-lock.json
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-4347
📔 Objective
⏰ Reminders before review
🦮 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