[HoT][Bling] Populate Identity Docs with entries [chromium/src : main]
Julia Sobiech (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Vidhan Jain
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vidhan Jain (Gerrit)
Vidhan Jain added 7 comments![]( "Open in Gerrit")
applicationLocale:locale];The locale can be fetched directly in the mediator since it's not used anywhere else.
raw_ptr<autofill::EntityDataManager> _entityDataManager;
std::unique_ptr<autofill::IOSAutofillEntityDataManagerObserverBridge>
_entityDataManagerObserver;
std::string _applicationLocale;Could you add comments?
if ([item isKindOfClass:[AutofillAIEntityItem class]]) {You can streamline this by using `base::apple::ObjCCast`, which internally performs the class check and safely returns `nil` if the type does not match.
```objc
AutofillAIEntityItem* aiItem =
base::apple::ObjCCast<AutofillAIEntityItem>(item);
if (aiItem) {
return aiItem.guid;
}
return std::nullopt;
```
symbolName = kPersonTextRectangleSymbol;Is it same as the driver's license icon?
if (_identityDocsItems.count > 0) {What happens if the user deletes all the items? Is a blank view shown?
[self reloadData];`reloadData` internally calls `loadModel`.
https://source.chromium.org/chromium/chromium/src/+/main:ios/chrome/browser/settings/ui_bundled/settings_root_table_view_controller.mm;l=136
[self.delegate identityDocsTableViewController:self didSelectItem:item];Currently, the flow is:
UI -> tells Coordinator (user tapped item) -> queries Mediator (what is the ID for this item?) -> Coordinator (navigates).
I think a better approach is to use the mutator pattern.
ViewController can send the tap action to a Mutator (the Mediator), which then extracts the ID and commands its delegate (the Coordinator) to navigate.
WDYT?
Related details
- Julia Sobiech
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vidhan Jain (Gerrit)
Vidhan Jain added 1 comment![]( "Open in Gerrit")
Could you also add some unit tests?
Julia Sobiech (Gerrit)
Julia Sobiech added 8 comments![]( "Open in Gerrit")
Could you also add some unit tests?
Done, since it currently only displays the data created elsewhere, more tests will be added as new functionalities are introduced within the page.
The locale can be fetched directly in the mediator since it's not used anywhere else.
Done
raw_ptr<autofill::EntityDataManager> _entityDataManager;
std::unique_ptr<autofill::IOSAutofillEntityDataManagerObserverBridge>
_entityDataManagerObserver;
std::string _applicationLocale;Julia SobiechCould you add comments?
Done!
if ([item isKindOfClass:[AutofillAIEntityItem class]]) {You can streamline this by using `base::apple::ObjCCast`, which internally performs the class check and safely returns `nil` if the type does not match.
```objc
AutofillAIEntityItem* aiItem =
base::apple::ObjCCast<AutofillAIEntityItem>(item);
if (aiItem) {
return aiItem.guid;
}
return std::nullopt;
```
Done, logic altered after the introduction of the Mediator
Is it same as the driver's license icon?
Yes, the icons should be the same. I simplified the switch statement to reflect that.
What happens if the user deletes all the items? Is a blank view shown?
Yes, at a later stage, data will be divided by types but to align with the present logic in addresses, we won't show empty titles or tables when no data is present. Currently we're left with an empty page: https://screenshot.googleplex.com/3JycYAnDLy7QEQx.png
`reloadData` internally calls `loadModel`.
https://source.chromium.org/chromium/chromium/src/+/main:ios/chrome/browser/settings/ui_bundled/settings_root_table_view_controller.mm;l=136
Done, thanks!
[self.delegate identityDocsTableViewController:self didSelectItem:item];Currently, the flow is:
UI -> tells Coordinator (user tapped item) -> queries Mediator (what is the ID for this item?) -> Coordinator (navigates).I think a better approach is to use the mutator pattern.
ViewController can send the tap action to a Mutator (the Mediator), which then extracts the ID and commands its delegate (the Coordinator) to navigate.WDYT?
Great, I think it indeed makes more sense as the encapsulation is stronger and clearer.
Related details
- Vidhan Jain
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |