New issue
Advanced search Search tips

Issue 825017 link

Starred by 1 user
Status: Fixed
Owner:
thegreenfrog@chromium.org
Closed: Jun 2018
Cc:
sczs@chromium.org
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: 2018-05-09
OS: iOS
Pri: 2
Type: Task
Q2

Blocking:
issue 822988



Sign in to add a comment

Collections: Polish URL cells

Project Member Reported by rohitrao@chromium.org, Mar 22 2018 
Specs will be produced in  Issue 825016 .

This will involve:
- Fixing margins and spacing for all the labels and image views inside the cell.
- Setting the proper font sizes for labels, ideally using dynamic type.
- Fetching favicons and drawing the rounded rect favicon background.
- (Optionally) Implementing a vertical layout for the accessibility content sizes.

Comment 1 by thegreenfrog@chromium.org, Mar 22 2018

Owner: thegreenfrog@chromium.org

Comment 2 by thegreenfrog@chromium.org, Mar 27 2018

Status: Started (was: Assigned)

Comment 3 by khalilcader@chromium.org, Mar 27 2018

Blockedon: -825016

Comment 4 by thegreenfrog@chromium.org, Mar 27 2018 

Starting work on this bug. I'll try to break this CL bug into three CLs.

1. Section Headers (generic_section_header_view margins and icon removal)
2. Recently Closed Tab Cell (session_data_tab_view margins and setting dynamic font sizes for labels that also implement vertical layout for accessibility content sizes)
3. Title & Done Button (RecentTabsHandsetViewController changes)

Comment 5 by thegreenfrog@chromium.org, Mar 27 2018 

Realized that I was changing legacy code. Forgot to turn on flags in my device!

1. Section Headers (add chevron icon and change font)
2. Recently Closed Tab Cell (add favicon and set dynamic font sizes)
3. Done already

Comment 6 by cmasso@google.com, Apr 5 2018

Labels: Pri-2
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 5 2018 

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

commit b1d6cfe1835cd0fd70cfd283efd89afd67e4c0b9
Author: Chris Lu <thegreenfrog@google.com>
Date: Thu Apr 05 22:36:18 2018

[ios] Add disclosure accessory view to Recent Tabs section headers.

Only adds the accessory. No rotation happens when toggling between collapsing/showing the sections.
Rotation will be in future CL.

Bug:  825017 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Idca5bd47ca06db6ef394b40690e2c3dd0c83345b
Reviewed-on: https://chromium-review.googlesource.com/987094
Commit-Queue: Chris Lu <thegreenfrog@google.com>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548593}
[modify] https://crrev.com/b1d6cfe1835cd0fd70cfd283efd89afd67e4c0b9/ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.mm
[modify] https://crrev.com/b1d6cfe1835cd0fd70cfd283efd89afd67e4c0b9/ios/chrome/browser/ui/table_view/cells/BUILD.gn
[modify] https://crrev.com/b1d6cfe1835cd0fd70cfd283efd89afd67e4c0b9/ios/chrome/browser/ui/table_view/cells/resources/table_view_cell_chevron.imageset/Contents.json
[add] https://crrev.com/b1d6cfe1835cd0fd70cfd283efd89afd67e4c0b9/ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.h
[add] https://crrev.com/b1d6cfe1835cd0fd70cfd283efd89afd67e4c0b9/ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.mm
[add] https://crrev.com/b1d6cfe1835cd0fd70cfd283efd89afd67e4c0b9/ios/chrome/browser/ui/table_view/cells/table_view_disclosure_header_footer_item.h
[add] https://crrev.com/b1d6cfe1835cd0fd70cfd283efd89afd67e4c0b9/ios/chrome/browser/ui/table_view/cells/table_view_disclosure_header_footer_item.mm
[modify] https://crrev.com/b1d6cfe1835cd0fd70cfd283efd89afd67e4c0b9/ios/chrome/browser/ui/table_view/cells/table_view_text_header_footer_item.mm

Comment 8 by cmasso@google.com, Apr 10 2018

Labels: -Type-Bug Type-Task
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 18 2018 

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

commit 8a8a1caecddd95b395e7428f3d83e5ccd48321ec
Author: Chris Lu <thegreenfrog@google.com>
Date: Wed Apr 18 16:23:13 2018

[ios] Add activity indicator to the Other Devices Loading section header in Recent Tabs.

Creates new HeaderFooterClass for this section header. Also moves the "Syncing..." text to the section header subtitle and removes the cell that originally displayed that text.

Video: https://drive.google.com/file/d/1jP5uSrm1MUyZVDMFPntQanqSe0hWh7GL/view?usp=sharing

Bug:  825017 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If2343c98adbaef6fee9ed7c851291d92a1b63502
Reviewed-on: https://chromium-review.googlesource.com/1011351
Commit-Queue: Chris Lu <thegreenfrog@google.com>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551710}
[modify] https://crrev.com/8a8a1caecddd95b395e7428f3d83e5ccd48321ec/ios/chrome/app/strings/ios_strings.grd
[modify] https://crrev.com/8a8a1caecddd95b395e7428f3d83e5ccd48321ec/ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.mm
[modify] https://crrev.com/8a8a1caecddd95b395e7428f3d83e5ccd48321ec/ios/chrome/browser/ui/table_view/cells/BUILD.gn
[add] https://crrev.com/8a8a1caecddd95b395e7428f3d83e5ccd48321ec/ios/chrome/browser/ui/table_view/cells/table_view_activity_indicator_header_footer_item.h
[add] https://crrev.com/8a8a1caecddd95b395e7428f3d83e5ccd48321ec/ios/chrome/browser/ui/table_view/cells/table_view_activity_indicator_header_footer_item.mm
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 25 2018 

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

commit 82b9f92f1212259de09089cdfc52d1e03907f4f0
Author: Chris Lu <thegreenfrog@chromium.org>
Date: Wed Apr 25 15:50:58 2018

[ios] Recent Tabs Table View Headers, TableViewUrlCell, and Table  polishing

Fix the following font, stying, and layouts based on these specs: https://folio.googleplex.com/ntp/Recent%20Tabs#%2Frecent_tabs.html
- Slimmed down spacing between stackviews and their container view.
- Changed text color of cell subtitle and cellseparatorinset.
- Made DisclosureViewHeader description label a dynamic font type.

Accessibility dynamic font is present, but changing the layout of cell contents to a vertical stack will come in future CL.

Before: https://drive.google.com/file/d/1f_lz35oaTzKXfl6J6lnt820eHHYIM_AP/view?usp=sharing
After: https://drive.google.com/file/d/1-nELR8ELhZuPmeYioPWKZVzqj3ynW8hV/view?usp=sharing

Dynamic Types:
iPad
   Regular - https://drive.google.com/file/d/1h5ej1hfmIscYx6xs7W0VuwzA15RFjQ95/view?usp=sharing
             https://drive.google.com/file/d/1qt3XEb7UornuV3q6GGxM6WCxaUf7cxdz/view?usp=sharing
   Large   - https://drive.google.com/file/d/1tV186qR4HI82dbYNioVICOhz1CoFYQcF/view?usp=sharing
             https://drive.google.com/file/d/1flOduXZUwGjGMR2hjPjNlAet74dgtS69/view?usp=sharing
iPhone
   regular - https://drive.google.com/file/d/1e_PvBbXyqFrOoJhEHKBXGJ2FAgyKZ0Vw/view?usp=sharing
             https://drive.google.com/file/d/1TSQJs-FWa0V98EEKvJLw8sEL9gQXJNXd/view?usp=sharing
   Large   - https://drive.google.com/file/d/11wXakU1vDcn2-bT8IcF_fnnnAD2O9K5t/view?usp=sharing
             https://drive.google.com/file/d/1bIKGp5zuxG8BrYV_CtTeQo38cwz_k99t/view


Bug:  825017 
Change-Id: I8f82cfaebac453b898e60cb6afb9b887754f527a
Reviewed-on: https://chromium-review.googlesource.com/1017918
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553575}
[modify] https://crrev.com/82b9f92f1212259de09089cdfc52d1e03907f4f0/ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.mm
[modify] https://crrev.com/82b9f92f1212259de09089cdfc52d1e03907f4f0/ios/chrome/browser/ui/table_view/cells/table_view_activity_indicator_header_footer_item.mm
[modify] https://crrev.com/82b9f92f1212259de09089cdfc52d1e03907f4f0/ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.h
[modify] https://crrev.com/82b9f92f1212259de09089cdfc52d1e03907f4f0/ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.mm
[modify] https://crrev.com/82b9f92f1212259de09089cdfc52d1e03907f4f0/ios/chrome/browser/ui/table_view/cells/table_view_disclosure_header_footer_item.mm
[modify] https://crrev.com/82b9f92f1212259de09089cdfc52d1e03907f4f0/ios/chrome/browser/ui/table_view/cells/table_view_text_header_footer_item.mm
[modify] https://crrev.com/82b9f92f1212259de09089cdfc52d1e03907f4f0/ios/chrome/browser/ui/table_view/cells/table_view_url_item.mm
[modify] https://crrev.com/82b9f92f1212259de09089cdfc52d1e03907f4f0/ios/chrome/browser/ui/table_view/chrome_table_view_controller.mm

Comment 12 by thegreenfrog@chromium.org, Apr 25 2018

NextAction: 2018-05-09

Comment 13 by monor...@bugs.chromium.org, May 9 2018 

The NextAction date has arrived: 2018-05-09

Comment 14 by thegreenfrog@chromium.org, Jun 22 2018

Status: Fixed (was: Started)
►

Sign in to add a comment