-
Notifications
You must be signed in to change notification settings - Fork 934
[PM-31393] Sends: UI/UX inconsistency of the password field #6435
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for your contribution! We've added this to our internal tracking system for review. Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process. |
b1e353a to
300cbdf
Compare
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt
Outdated
Show resolved
Hide resolved
1c25f52 to
1f9e835
Compare
| it.copy( | ||
| passwordInput = ( | ||
| action.generatorResult as GeneratorResult.Password | ||
| ).password, |
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.
Can we make this a little safer:
(action.generatorResult as? GeneratorResult.Password)?.let { passwordData ->
updateCommonContent {
it.copy(passwordInput = passwordData.password)
}
}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.
@david-livefront done!
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendViewModel.kt
Outdated
Show resolved
Hide resolved
1f9e835 to
0b42311
Compare
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/SendViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/com/x8bit/bitwarden/ui/tools/feature/send/addedit/AddEditSendContent.kt
Outdated
Show resolved
Hide resolved
0b42311 to
8baa08d
Compare
|
@shamim-emon Our design and product teams are reviewing the screen changes. In the meantime, can you update tests to cover these changes? |
8baa08d to
10572cd
Compare
77dc323 to
f0cf171
Compare
@SaintPatrck I have added test coverage for these changes. Please have a look and let me know if anything else is needed from my side. |
f0cf171 to
858cc0d
Compare
🎟️ Tracking
#6335
📔 Objective
Adds the ability to generate and copy passwords under the Send tab.
📸 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