-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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-6819] add legacyUsernameGenerationService
to dependency injection
#8606
[PM-6819] add legacyUsernameGenerationService
to dependency injection
#8606
Conversation
UsernameGeneratorService
with `legacyUsernameGenerationServ…UsernameGeneratorService
with legacyUsernameGenerationService
UsernameGeneratorService
with legacyUsernameGenerationService
legacyUsernameGenerationService
to dependency injection
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as resolved.
This comment was marked as resolved.
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.
Great work!
Just a question regarding when to clear the state.
Will hold the approval until #8706 is merged
"addyIoBuffer", | ||
{ | ||
deserializer: (value) => value, | ||
clearOn: ["lock"], |
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.
is ClearOn
lock
correct? If it is, should it also clear on logout
?
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 you're right, and this should only occur on logout. Technically, it should rarely matter. Those buffers clear themselves automatically.
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.
Addressed in e40d57c
This comment was marked as outdated.
This comment was marked as outdated.
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.
Such an improved way of storing this data!
Props to @djsmith85 for doing the v1 iteration of the options types. His version made one options type per service. I might go back to it since the destructured approach isn't actually used by the forwarders, which makes it kind of noisy. Either way, splitting it out into forwarder-specific states is 100% a boon. |
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.
Great work on this!
Withdrawing this PR because the generator UI needs to be fully reactive. This branch will be merged into a single branch that includes it, #8605, and reactive UI updates for the password generator. |
Type of change
Depends on #8596 and #8706
Objective
Integrate username generator adapter into bitwarden clients.
Code changes
Updates dependency injection of all clients from the state provider backed password generator service to MV3 backed password generator adapter:
Data migration:
Add rollover keys to generator strategies:
Publish adapter factory from generator module:
Fix missing default navigation settings: