New issue
Advanced search Search tips

Issue 721758 link

Starred by 1 user
Status: Fixed
Owner:
gambard@chromium.org
Closed: Jun 2017
Cc:
gambard@chromium.org
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

Make ReadingList UI compatible with the new architecture

Project Member Reported by gambard@chromium.org, May 12 2017 
This include:
- Removing all dependencies on components/
- Removing the use of GURL

To do this a mediator class will be created and will handle the ReadingListModel. The ReadingListCollectionItem will be created in the mediator and offered to the ViewController.
Project Member

Comment 1 by bugdroid1@chromium.org, May 16 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c5ee3cba18800bf7664aaef43afd379e5c9a217

commit 0c5ee3cba18800bf7664aaef43afd379e5c9a217
Author: gambard <gambard@chromium.org>
Date: Tue May 16 20:30:13 2017

Move the ReadingListModel in a Mediator

The ReadingListMediator will now handle the ReadingListModel instead of
passing it directly in the ReadingListCollectionViewController.

BUG= 721758 ,  688392 

Review-Url: https://codereview.chromium.org/2879183002
Cr-Commit-Position: refs/heads/master@{#472205}

[modify] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/BUILD.gn
[modify] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h
[modify] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
[modify] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller_unittest.mm
[modify] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm
[modify] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/reading_list_coordinator_unittest.mm
[add] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/reading_list_data_sink.h
[add] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/reading_list_data_source.h
[add] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/reading_list_mediator.h
[add] https://crrev.com/0c5ee3cba18800bf7664aaef43afd379e5c9a217/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9e02c078fa310d0169787bb7a2187384ee4fc18

commit b9e02c078fa310d0169787bb7a2187384ee4fc18
Author: gambard <gambard@chromium.org>
Date: Tue Jun 06 14:29:15 2017

ReadingListDataSource returns items instead of Entries

The ReadingListEntry are used in the model. The UI layer should not depend on
the ReadingListEntry. This CL move the creation of the ReadingListItems from
the view controller to the mediator.

BUG= 721758 ,  688392 

Review-Url: https://codereview.chromium.org/2885383002
Cr-Commit-Position: refs/heads/master@{#477281}

[modify] https://crrev.com/b9e02c078fa310d0169787bb7a2187384ee4fc18/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
[modify] https://crrev.com/b9e02c078fa310d0169787bb7a2187384ee4fc18/ios/chrome/browser/ui/reading_list/reading_list_data_source.h
[modify] https://crrev.com/b9e02c078fa310d0169787bb7a2187384ee4fc18/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ff947cea6682a0cc4b8e7578b4d43534140e843

commit 0ff947cea6682a0cc4b8e7578b4d43534140e843
Author: gambard <gambard@chromium.org>
Date: Tue Jun 06 18:07:12 2017

Add test for ReadingListMediator

Test ReadingListMediator for filling read and unread array.

BUG= 721758 

Review-Url: https://codereview.chromium.org/2917363004
Cr-Commit-Position: refs/heads/master@{#477341}

[modify] https://crrev.com/0ff947cea6682a0cc4b8e7578b4d43534140e843/ios/chrome/browser/ui/reading_list/BUILD.gn
[add] https://crrev.com/0ff947cea6682a0cc4b8e7578b4d43534140e843/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm

Comment 4 by gambard@chromium.org, Jun 8 2017

Cc: gambard@chromium.org
 Issue 688392  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 8 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21c0c8b88856163c64118ae44e27c102541624ee

commit 21c0c8b88856163c64118ae44e27c102541624ee
Author: gambard <gambard@chromium.org>
Date: Thu Jun 08 14:24:01 2017

Separate model from UI for ReadingList accessibility

The accessibility quick actions of Reading List mixed UI and model. This CL
separates both.

BUG= 721758 

Change-Id: I809feec247f8f73ed7924df6f7d49371a06c956c
Reviewed-on: https://chromium-review.googlesource.com/527439
Reviewed-by: Louis Romero <lpromero@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477959}
[modify] https://crrev.com/21c0c8b88856163c64118ae44e27c102541624ee/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h
[modify] https://crrev.com/21c0c8b88856163c64118ae44e27c102541624ee/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
[modify] https://crrev.com/21c0c8b88856163c64118ae44e27c102541624ee/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.mm
[modify] https://crrev.com/21c0c8b88856163c64118ae44e27c102541624ee/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item_accessibility_delegate.h
[modify] https://crrev.com/21c0c8b88856163c64118ae44e27c102541624ee/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm
[modify] https://crrev.com/21c0c8b88856163c64118ae44e27c102541624ee/ios/chrome/browser/ui/reading_list/reading_list_data_source.h
[modify] https://crrev.com/21c0c8b88856163c64118ae44e27c102541624ee/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da13d749718a502bb113d962fe32c66bbec6f35c

commit da13d749718a502bb113d962fe32c66bbec6f35c
Author: gambard <gambard@chromium.org>
Date: Fri Jun 09 10:53:43 2017

Move the model observing to the mediator

The Reading List collection should only be notified when it should reload.
This CL moves the logic to know if the model has changed during a sync to the
ReadingListMediator.

BUG= 721758 

Change-Id: Ifc9cd15ff7065776732ab29bd6cf3ce4499ad713
Reviewed-on: https://chromium-review.googlesource.com/528114
Reviewed-by: Jean-François Geyelin <jif@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478244}
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/BUILD.gn
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller_unittest.mm
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_coordinator_unittest.mm
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_data_sink.h
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_data_source.h
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_mediator.h
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
[modify] https://crrev.com/da13d749718a502bb113d962fe32c66bbec6f35c/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4340ea66a6dee14b9bdbdccacf735aa1df428e7

commit f4340ea66a6dee14b9bdbdccacf735aa1df428e7
Author: gambard <gambard@chromium.org>
Date: Fri Jun 09 11:13:05 2017

Split the ReadingListUI target in two

This CL splits the Reading List target in a UI target and a coordinator
target.
For now the ViewController are part of the coordinator target to have a clean
UI target. Once their are refactored, they will be part of the UI target.

BUG= 721758 

Change-Id: Id201753fa6f8bc8cc673039d940c57cc4d16d068
Reviewed-on: https://chromium-review.googlesource.com/527438
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478247}
[modify] https://crrev.com/f4340ea66a6dee14b9bdbdccacf735aa1df428e7/ios/chrome/browser/ui/reading_list/BUILD.gn
[add] https://crrev.com/f4340ea66a6dee14b9bdbdccacf735aa1df428e7/ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.h
[add] https://crrev.com/f4340ea66a6dee14b9bdbdccacf735aa1df428e7/ios/chrome/browser/ui/reading_list/reading_list_collection_view_cell.mm
[modify] https://crrev.com/f4340ea66a6dee14b9bdbdccacf735aa1df428e7/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
[modify] https://crrev.com/f4340ea66a6dee14b9bdbdccacf735aa1df428e7/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.h
[modify] https://crrev.com/f4340ea66a6dee14b9bdbdccacf735aa1df428e7/ios/chrome/browser/ui/reading_list/reading_list_collection_view_item.mm
[modify] https://crrev.com/f4340ea66a6dee14b9bdbdccacf735aa1df428e7/ios/chrome/browser/ui/reading_list/reading_list_data_source.h
[modify] https://crrev.com/f4340ea66a6dee14b9bdbdccacf735aa1df428e7/ios/chrome/browser/ui/reading_list/reading_list_utils.h
[modify] https://crrev.com/f4340ea66a6dee14b9bdbdccacf735aa1df428e7/ios/chrome/browser/ui/tools_menu/BUILD.gn
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79e65182ac5735c9ebe072dbaf2f19a10f1e6475

commit 79e65182ac5735c9ebe072dbaf2f19a10f1e6475
Author: gambard <gambard@chromium.org>
Date: Fri Jun 09 12:18:23 2017

ReadingListCVCDelegate takes generic CollectionViewItem

The ReadingListCollectionViewControllerDelegate takes generic
CollectionViewItem as argument.
The ReadingListCollectionViewItem are part of the coordinator layer, the VC
should not be aware of them.

BUG= 721758 

Change-Id: Idaf640162dd1f6494fc7359b0a6d5d4a899daf2e
Reviewed-on: https://chromium-review.googlesource.com/528234
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Louis Romero <lpromero@chromium.org>
Reviewed-by: Elodie Banel <lod@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478253}
[modify] https://crrev.com/79e65182ac5735c9ebe072dbaf2f19a10f1e6475/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.h
[modify] https://crrev.com/79e65182ac5735c9ebe072dbaf2f19a10f1e6475/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
[modify] https://crrev.com/79e65182ac5735c9ebe072dbaf2f19a10f1e6475/ios/chrome/browser/ui/reading_list/reading_list_coordinator.h
[modify] https://crrev.com/79e65182ac5735c9ebe072dbaf2f19a10f1e6475/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm
[modify] https://crrev.com/79e65182ac5735c9ebe072dbaf2f19a10f1e6475/ios/chrome/browser/ui/reading_list/reading_list_coordinator_unittest.mm
[modify] https://crrev.com/79e65182ac5735c9ebe072dbaf2f19a10f1e6475/ios/chrome/browser/ui/reading_list/reading_list_mediator.h
[modify] https://crrev.com/79e65182ac5735c9ebe072dbaf2f19a10f1e6475/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e8ca79558fc6b22196c1f82766e12bba3b10f87

commit 7e8ca79558fc6b22196c1f82766e12bba3b10f87
Author: gambard <gambard@chromium.org>
Date: Fri Jun 09 12:57:31 2017

Remove the GURL from ReadingListCollectionVC

The ReadingListCollectionViewController should not be aware of the GURL as part
of the UI layer. This CL removes the usage of GURL by changing the way the
entry are updated.

BUG= 721758 

Change-Id: I496c307c8978230caa29160ca6eb005afc5a37f3
Reviewed-on: https://chromium-review.googlesource.com/528099
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Elodie Banel <lod@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478257}
[modify] https://crrev.com/7e8ca79558fc6b22196c1f82766e12bba3b10f87/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
[modify] https://crrev.com/7e8ca79558fc6b22196c1f82766e12bba3b10f87/ios/chrome/browser/ui/reading_list/reading_list_coordinator.mm
[modify] https://crrev.com/7e8ca79558fc6b22196c1f82766e12bba3b10f87/ios/chrome/browser/ui/reading_list/reading_list_data_source.h
[modify] https://crrev.com/7e8ca79558fc6b22196c1f82766e12bba3b10f87/ios/chrome/browser/ui/reading_list/reading_list_mediator.h
[modify] https://crrev.com/7e8ca79558fc6b22196c1f82766e12bba3b10f87/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 9 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5424b28fe642fc2c3632e4adebfb5483f8df9123

commit 5424b28fe642fc2c3632e4adebfb5483f8df9123
Author: gambard <gambard@chromium.org>
Date: Fri Jun 09 14:50:48 2017

Use generic item in the ReadingListCollectionVC

The ReadingListCollectionViewController now use only CollectionViewItem.
This allows the ReadingListCollectionVC and the ReadingListVC to move to the
UI target, giving a layer separation.

BUG= 721758 

Change-Id: I43690c74d493ecd6f6c2ec653035da9f60674497
Reviewed-on: https://chromium-review.googlesource.com/529126
Reviewed-by: Louis Romero <lpromero@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478274}
[modify] https://crrev.com/5424b28fe642fc2c3632e4adebfb5483f8df9123/ios/chrome/browser/content_suggestions/BUILD.gn
[modify] https://crrev.com/5424b28fe642fc2c3632e4adebfb5483f8df9123/ios/chrome/browser/ui/reading_list/BUILD.gn
[modify] https://crrev.com/5424b28fe642fc2c3632e4adebfb5483f8df9123/ios/chrome/browser/ui/reading_list/reading_list_collection_view_controller.mm
[modify] https://crrev.com/5424b28fe642fc2c3632e4adebfb5483f8df9123/ios/chrome/browser/ui/reading_list/reading_list_data_source.h
[modify] https://crrev.com/5424b28fe642fc2c3632e4adebfb5483f8df9123/ios/chrome/browser/ui/reading_list/reading_list_mediator.mm
[modify] https://crrev.com/5424b28fe642fc2c3632e4adebfb5483f8df9123/ios/chrome/browser/ui/reading_list/reading_list_mediator_unittest.mm

Comment 11 by gambard@chromium.org, Jun 12 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment