firefox-ios
firefox-ios copied to clipboard
Add FXIOS-9201 ⁃ Enhanced Tracking Protection certificates details screen
:scroll: Tickets
:bulb: Description
Added Tracking Protection Certificates screen
:pencil: Checklist
You have to check all boxes before merging
- [x] Filled in the above information (tickets numbers and description of your work)
- [x] Updated the PR name to follow our PR naming guidelines
- [ ] Wrote unit tests and/or ensured the tests suite is passing
- [ ] When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
- [ ] If needed, I updated documentation / comments for complex code and public methods
- [ ] If needed, added a backport comment (example
@Mergifyio backport release/v120)
| Messages | |
|---|---|
| :book: | Project coverage: 31.57% |
| :book: | Edited 4 files |
| :book: | Created 4 files |
Client.app: Coverage: 30.27
| File | Coverage | |
|---|---|---|
| TrackingProtectionViewController.swift | 28.69% | ⚠️ |
| CertificatesHandler.swift | 0.0% | ⚠️ |
| TrackingProtectionModel.swift | 29.55% | ⚠️ |
| CertificatesViewModel.swift | 71.43% | ✅ |
| CertificatesViewController.swift | 0.0% | ⚠️ |
| CertificatesCell.swift | 0.0% | ⚠️ |
Generated by :no_entry_sign: Danger Swift against fec4e51804a5c1ec31521cc2e55f4327969b8c7a
This pull request has conflicts when rebasing. Could you fix it @dicarobinho? 🙏
Sorry I didn't get a chance to review this on Friday @dicarobinho. Would you be able to provide instructions for how I can access this screen to test in the app? Thanks!
Sorry I didn't get a chance to review this on Friday @dicarobinho. Would you be able to provide instructions for how I can access this screen to test in the app? Thanks!
No, at this step is not possible without doing some changes locally.
@dicarobinho I haven't reviewed thoroughly yet, will give another look once @ih-codes comments are addressed. However, we will be using the certificate screen for the error pages as well. Is this view reusable in that case?
Please provide screenshots as well, it will help review. Thank you!
Screen is not done yet, so screenshots may not be useful at this step. (I have another TODO done for which I will create a PR after this one will be merged.)
@dicarobinho I haven't reviewed thoroughly yet, will give another look once @ih-codes comments are addressed. However, we will be using the certificate screen for the error pages as well. Is this view reusable in that case? Please provide screenshots as well, it will help review. Thank you!
Screen is not done yet, so screenshots may not be useful at this step. (I have another TODO done for which I will create a PR after this one will be merged.)
Even if the screen is not done, it'll be helpful to add it to represent what code is being merged from a user perspective. At least from my side, it'll help me provide a quicker review to visualize. :)
@dicarobinho I haven't reviewed thoroughly yet, will give another look once @ih-codes comments are addressed. However, we will be using the certificate screen for the error pages as well. Is this view reusable in that case? Please provide screenshots as well, it will help review. Thank you!
Screen is not done yet, so screenshots may not be useful at this step. (I have another TODO done for which I will create a PR after this one will be merged.)
Even if the screen is not done, it'll be helpful to add it to represent what code is being merged from a user perspective. At least from my side, it'll help me provide a quicker review to visualize. :)
Sorry I didn't get a chance to review this on Friday @dicarobinho. Would you be able to provide instructions for how I can access this screen to test in the app? Thanks!
No, at this step is not possible without doing some changes locally.
I don't mind making a few changes in my code so I can test the screen. But if it is complicated, then don't worry about it. The screenshot you posted helps! 👍 I imagine more detailed testing can be done once the screen is finished.