Code review comment for lp://staging/~renatofilho/address-book-app/sim-card-import

Revision history for this message
Bill Filler (bfiller) wrote :

A few issues:

1) The label on contact list page should say "Import contacts from SIM card" not "Import contacts from sim card"
2) There is currently a toolbar button (with arrow) on the first time contact list page (i.e. the page that displays "Import from Google", "Import from SIM card"). We should not show the toolbar button here as there is already an option in the list
3) The header title when doing import from sim should say "Import SIM Contacts" I think, as there is no indication where these are coming from
4) What happens in the dual-sim case if you have contacts on both sims? Does the view show all of them combined? (I would expect it to)
5) Is there a better icon we could get perhaps? This one looks like a Download/Save button to me
6) Wondering if we should always have the toolbar icon in a drawer? Whether it's by itself or with a sync button. This way we have text to describe what the action is and the icon becomes less import. Once we have settings we'll need a drawer probably anyay

review: Needs Fixing

« Back to merge proposal