[PM-14503] Add lang attr on desktop and browser
#14691
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #14691 +/- ##
==========================================
- Coverage 36.84% 36.84% -0.01%
==========================================
Files 3208 3210 +2
Lines 92719 92738 +19
Branches 13935 13935
==========================================
+ Hits 34164 34169 +5
- Misses 57150 57164 +14
Partials 1405 1405 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Great job, no security vulnerabilities found in this Pull Request |
| ) { | ||
| this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); | ||
|
|
||
| const langSubscription = this.documentLangSetter.start(); | ||
| this.destoryRef.onDestroy(() => langSubscription.unsubscribe()); |
There was a problem hiding this comment.
question: why not use onDestroy on the service instead?
There was a problem hiding this comment.
Oh that's because the service will keep on living but the app.component might have been destroyed. Does that ever actually happen? And if that is the case we could also add the service as a component-level provider instead so that we also destroy the service instead of just unsubscribing
There was a problem hiding this comment.
I could use onDestroy I would just need to stash the Subscription onto the class so that I can call this.langSubscription.unsubscribe() on it. This felt a little more encapsulated and generally it seems like we can avoid an explicit onDestroy if we use things like takeUntilDestroyed() which is using DestroyRef under the hood so this felt like the "new way" to me but I am not up to date with my angular best practices so I'm happy to change it if you'd prefer it.
There was a problem hiding this comment.
You should be able to depend on DestroyRef in your class and use it there too if you don't want to use onDestroy. I think both of those approaches are fine 👍
|
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-14503
📔 Objective
Adds a service to listen to
I18nServiceand apply thelangattribute to thedocument. This is then initialized in every applications app component.📸 Screenshots
⏰ 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