BIT-1003: Generate plus addressed emails #129
Conversation
matt-livefront
requested review from
fedemkr,
jubie-livefront,
nathan-livefront and
eliot-livefront
Bitwarden code coverageTotal coverage:
|
| File | Coverage |
|---|---|
| BitwardenShared/Core/Tools/Repositories/GeneratorRepository.swift | 100.00% |
| BitwardenShared/Core/Tools/Services/CryptoService.swift | 100.00% |
| BitwardenShared/Core/Tools/Services/RandomNumberGenerator.swift | 0.00% |
| BitwardenShared/UI/Tools/Generator/Generator/GeneratorAction.swift | 94.12% |
| BitwardenShared/UI/Tools/Generator/Generator/GeneratorProcessor.swift | 93.75% |
| BitwardenShared/UI/Tools/Generator/Generator/GeneratorView.swift | 90.27% |
Powered by Slather
Generated by 🚫 Danger
There was a problem hiding this comment.
🤔 I just wanted to let you know that there are some open PRs on the SDK repo for Password/Passphrase/Username generation so not sure if it's worth you add the logic for that given it's going to be replaced afterwards for the SDK version once it's merged.
Username Generator
Passphrase Generator
Password Generator
| guard let atIndex = email.firstIndex(of: "@"), | ||
| // Ensure '@' symbol isn't the first or last character. | ||
| atIndex > email.startIndex, | ||
| atIndex < email.index(before: email.endIndex) else { | ||
| return email | ||
| } |
There was a problem hiding this comment.
🤔 What do you think of moving this to?
There was a problem hiding this comment.
Are you proposing replacing this entire guard with the isValidEmail check? I think these do slightly different, yet similar things. But if we're trying to match the Xamarin app which has been driving our requirements, the isValidEmail only checks that the string contains an @ symbol and doesn't check that it isn't the first or last character.
There was a problem hiding this comment.
Alright, for matching purposes we can leave it like this and refactor in the future. Cause IMO wherever isValidEmail is used, when checking if it has a @ it shouldn't be neither the start nor the end of the string so if we'd replace the current isValidEmail logic it'd be an improvement. However you're right and let's leave it like this so it's equal to the Xamarin app.
matt-livefront
force-pushed
the
matt/BIT-410-generator-catch-all-email-ui
branch
from
faac7b9 to
496c260
Compare
Ah, I didn't realize this. The document that I have only lists password and passphrase generation as being implemented for us by the SDK. But that's great if we'll be getting username generation too. I think we can keep this for now and replace it once it's in the SDK. |
matt-livefront
force-pushed
the
matt/BIT-1003-generate-plus-email
branch
from
bbedf1a to
078d4ab
Compare
matt-livefront
force-pushed
the
matt/BIT-1003-generate-plus-email
branch
from
078d4ab to
0441168
Compare
|
No New Or Fixed Issues Found |
🎟️ Tracking
BIT-1003
🚧 Type of change
📔 Objective
This adds generating of plus addressed emails.
📋 Code changes
@FocusStateto determine when a text field has or loses focus.📸 Screenshots
Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-11-03.at.15.11.37.mp4
⏰ 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