New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 740133 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Feature



Sign in to add a comment

Tracking bug for TextBadgeView implementation

Project Member Reported by helenlyang@google.com, Jul 7 2017

Issue description

Implement TextBadgeView and integrate with Showcase.
This is for the User Education project.
 
Description: Show this description

Comment 2 Deleted

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 10 2017

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

commit ac8a93564a2560a5ec54673246c551e04e11ed02
Author: helenlyang <helenlyang@google.com>
Date: Mon Jul 10 17:13:24 2017

[ios showcase] Integrated TextBadgeView with Showcase.

This CL adds TextBadgeView to iOS Showcase. This involves adding a
text_badge_view folder to showcase, creating a glue view controller,
and modifying the Showcase configuration file and BUILD.gn files as
necessary.

Since TextBadgeView is currently a stub implementation, Showcase
displays a blank white screen.

BUG= 740133 .

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

[modify] https://crrev.com/ac8a93564a2560a5ec54673246c551e04e11ed02/ios/showcase/BUILD.gn
[modify] https://crrev.com/ac8a93564a2560a5ec54673246c551e04e11ed02/ios/showcase/core/showcase_model.mm
[add] https://crrev.com/ac8a93564a2560a5ec54673246c551e04e11ed02/ios/showcase/text_badge_view/BUILD.gn
[add] https://crrev.com/ac8a93564a2560a5ec54673246c551e04e11ed02/ios/showcase/text_badge_view/sc_text_badge_view_controller.h
[add] https://crrev.com/ac8a93564a2560a5ec54673246c551e04e11ed02/ios/showcase/text_badge_view/sc_text_badge_view_controller.mm
[add] https://crrev.com/ac8a93564a2560a5ec54673246c551e04e11ed02/ios/showcase/text_badge_view/sc_text_badge_view_egtest.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 10 2017

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

commit 912335bc3e0cf9a880286811d0f342c3a7f1c230
Author: michaeldo <michaeldo@chromium.org>
Date: Mon Jul 10 20:17:07 2017

Revert of [ios showcase] Integrated TextBadgeView with Showcase. (patchset #4 id:80001 of https://codereview.chromium.org/2967113003/ )

Reason for revert:
This CL breaks ios_showcase_egtests suite. PTAL at the failure here: https://build.chromium.org/p/chromium.fyi/builders/EarlGreyiOS/builds/29530

and fix:
- SCToolbarTestCase/testRotationSizeClassChange
-
 SCToolbarTestCase/testVerifyToolbarButtonsLabelAndAction

then re-land.

Original issue's description:
> [ios showcase] Integrated TextBadgeView with Showcase.
>
> This CL adds TextBadgeView to iOS Showcase. This involves adding a
> text_badge_view folder to showcase, creating a glue view controller,
> and modifying the Showcase configuration file and BUILD.gn files as
> necessary.
>
> Since TextBadgeView is currently a stub implementation, Showcase
> displays a blank white screen.
>
> BUG= 740133 .
>
> Review-Url: https://codereview.chromium.org/2967113003
> Cr-Commit-Position: refs/heads/master@{#485302}
> Committed: https://chromium.googlesource.com/chromium/src/+/ac8a93564a2560a5ec54673246c551e04e11ed02

TBR=edchin@chromium.org,gchatz@chromium.org,helenlyang@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 740133 .

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

[modify] https://crrev.com/912335bc3e0cf9a880286811d0f342c3a7f1c230/ios/showcase/BUILD.gn
[modify] https://crrev.com/912335bc3e0cf9a880286811d0f342c3a7f1c230/ios/showcase/core/showcase_model.mm
[delete] https://crrev.com/6255080957159e8a6b341b026bbc6d2668375455/ios/showcase/text_badge_view/BUILD.gn
[delete] https://crrev.com/6255080957159e8a6b341b026bbc6d2668375455/ios/showcase/text_badge_view/sc_text_badge_view_controller.h
[delete] https://crrev.com/6255080957159e8a6b341b026bbc6d2668375455/ios/showcase/text_badge_view/sc_text_badge_view_controller.mm
[delete] https://crrev.com/6255080957159e8a6b341b026bbc6d2668375455/ios/showcase/text_badge_view/sc_text_badge_view_egtest.mm

Comment 5 by edchin@chromium.org, Jul 10 2017

Cc: gch...@chromium.org edchin@chromium.org
Status: Assigned (was: Untriaged)
Components: UI>Browser>Core
Labels: -Type-Bug Type-Feature
Ed, could you please update Component when assigning the bug. Thanks!
Project Member

Comment 7 by sheriffbot@chromium.org, Jul 11 2017

Labels: Hotlist-Google
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 13 2017

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

commit 0a8ba8f9f932955158bdec4df468bb03663a8e2c
Author: Helen Yang <helenlyang@google.com>
Date: Thu Jul 13 17:14:03 2017

Reland of [ios showcase] Integrated TextBadgeView with Showcase

The CL [ios showcase] Integrated TextBadgeView with Showcase 
(patchset #4 id:80001 of https://codereview.chromium.org/2967113003/)
broke ios_showcase_egtests suite for iPad. The revert can be found
here: https://codereview.chromium.org/2978533002/.

This CL is a reland of the first, and fixes the egtest issue by
ensuring that EarlGrey only selects sufficiently visible elements.

Bug:  740133 
Change-Id: Ie30b78e85d965bd2ff48d27fef4eb7f824d56d98
Reviewed-on: https://chromium-review.googlesource.com/567129
Commit-Queue: Helen Yang <helenlyang@google.com>
Reviewed-by: Ed Chin <edchin@chromium.org>
Reviewed-by: Gregory Chatzinoff <gchatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486420}
[modify] https://crrev.com/0a8ba8f9f932955158bdec4df468bb03663a8e2c/ios/showcase/BUILD.gn
[modify] https://crrev.com/0a8ba8f9f932955158bdec4df468bb03663a8e2c/ios/showcase/core/showcase_model.mm
[modify] https://crrev.com/0a8ba8f9f932955158bdec4df468bb03663a8e2c/ios/showcase/test/showcase_eg_utils.mm
[add] https://crrev.com/0a8ba8f9f932955158bdec4df468bb03663a8e2c/ios/showcase/text_badge_view/BUILD.gn
[add] https://crrev.com/0a8ba8f9f932955158bdec4df468bb03663a8e2c/ios/showcase/text_badge_view/sc_text_badge_view_controller.h
[add] https://crrev.com/0a8ba8f9f932955158bdec4df468bb03663a8e2c/ios/showcase/text_badge_view/sc_text_badge_view_controller.mm
[add] https://crrev.com/0a8ba8f9f932955158bdec4df468bb03663a8e2c/ios/showcase/text_badge_view/sc_text_badge_view_egtest.mm

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 26 2017

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

commit 9b9a4008572a4f69ff0fad0f80afa0d2d1e1c611
Author: Helen Yang <helenlyang@google.com>
Date: Wed Jul 26 22:08:51 2017

[ios] Implement TextBadgeView and integrate with Showcase

This CL implements TextBadgeView. It sets the appearance of the
badge and adds constraints so that subviews are centered and the
badge stretches to fit display text. Additionally, this CL
modifies the Showcase implementation so that it displays the text
badge instead of a placeholder rectangle. This commit does not
refactor NumberBadgeView.

Bug:  740133 
Change-Id: I8d5db930e725e273405ce5dd580b91f38a97fc09
Reviewed-on: https://chromium-review.googlesource.com/581728
Commit-Queue: Helen Yang <helenlyang@google.com>
Reviewed-by: Ed Chin <edchin@chromium.org>
Reviewed-by: Gregory Chatzinoff <gchatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489772}
[modify] https://crrev.com/9b9a4008572a4f69ff0fad0f80afa0d2d1e1c611/ios/chrome/browser/ui/reading_list/text_badge_view.h
[modify] https://crrev.com/9b9a4008572a4f69ff0fad0f80afa0d2d1e1c611/ios/chrome/browser/ui/reading_list/text_badge_view.mm
[modify] https://crrev.com/9b9a4008572a4f69ff0fad0f80afa0d2d1e1c611/ios/chrome/browser/ui/reading_list/text_badge_view_unittest.mm
[modify] https://crrev.com/9b9a4008572a4f69ff0fad0f80afa0d2d1e1c611/ios/showcase/core/showcase_model.mm
[modify] https://crrev.com/9b9a4008572a4f69ff0fad0f80afa0d2d1e1c611/ios/showcase/text_badge_view/sc_text_badge_view_controller.mm
[modify] https://crrev.com/9b9a4008572a4f69ff0fad0f80afa0d2d1e1c611/ios/showcase/text_badge_view/sc_text_badge_view_egtest.mm

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 10 2017

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

commit ccebb7a4df587ae86fa4dabaa3e032c4d75a8fec
Author: Helen Yang <helenlyang@google.com>
Date: Thu Aug 10 18:14:17 2017

[ios] Refactor NumberBadgeView and fix TextBadgeView bug

This CL refactors NumberBadgeView by replacing the background view
and label properties with a TextBadgeView property.
Since NumberBadgeView and TextBadgeView are similar in appearance,
this reduces the amount of repeated code. The refactoring also moves
calls to instance methods and property accessors from the
NumberBadgeView constructor to -willMoveToSuperview.

Secondly, this CL fixes a bug that caused TextBadgeView to stretch
across the reading list menu cell. It modifies constraints defined
in TextBadgeView and ReadingListMenuViewCell. Additionally, in
order to allow NumberBadgeView to set the width of the badge, this
CL adds a |labelHorizontalMargin| parameter to the designated
initializer. This is necessary because NumberBadgeView has a
smaller label margin than TextBadgeView. Finally, this CL moves calls
to instance methods and accessors from the constructor to
-willMoveToSuperview.

Bug:  740133 
Change-Id: Idc45cdf510aa1f5a362b40f494cfb9464c202662
Reviewed-on: https://chromium-review.googlesource.com/607571
Commit-Queue: Helen Yang <helenlyang@google.com>
Reviewed-by: Ed Chin <edchin@chromium.org>
Reviewed-by: Gregory Chatzinoff <gchatz@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493463}
[modify] https://crrev.com/ccebb7a4df587ae86fa4dabaa3e032c4d75a8fec/ios/chrome/browser/ui/reading_list/number_badge_view.mm
[modify] https://crrev.com/ccebb7a4df587ae86fa4dabaa3e032c4d75a8fec/ios/chrome/browser/ui/reading_list/text_badge_view.h
[modify] https://crrev.com/ccebb7a4df587ae86fa4dabaa3e032c4d75a8fec/ios/chrome/browser/ui/reading_list/text_badge_view.mm
[modify] https://crrev.com/ccebb7a4df587ae86fa4dabaa3e032c4d75a8fec/ios/chrome/browser/ui/reading_list/text_badge_view_unittest.mm
[modify] https://crrev.com/ccebb7a4df587ae86fa4dabaa3e032c4d75a8fec/ios/chrome/browser/ui/tools_menu/reading_list_menu_view_item.mm

Project Member

Comment 11 by sheriffbot@chromium.org, Aug 15 2017

Labels: Hotlist-Recharge-BouncingOwner
Owner: ----
Status: Untriaged (was: Assigned)
The assigned owner "helenlyang@google.com" is not able to receive e-mails, please re-triage.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Owner: helenlyang@google.com
Status: Fixed (was: Untriaged)

Sign in to add a comment