New issue
Advanced search Search tips

Issue 843571 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Task

Blocked on:
issue 817753



Sign in to add a comment

UX Request: Infobar Clarifications

Project Member Reported by rohitrao@chromium.org, May 16 2018

Issue description

Regarding the attached mock: 

1. Rounded corner radius for the image background is 12px as shown. Is that correct?

2. Rounded corner radius for the ""Restore"" button is 11px as shown. Is that correct?

3. Please review the font used. The body text is [UIFont preferredFontForTextStyle:UIFontTextStyleSubheadline] and the button label is [UIFont preferredFontForTextStyle:UIFontTextStyleHeadline].

4. Do we need a new icon or is the ""sad folder"" used in Bling going to be used in Bijou as well.

5. The small dismiss ""x"" for Bijou at the top right is about 2/3 the size of that used in Bling. I need the image assets in PNG for all 3 resolutions. FYI, the Bling image is 24x24 in dimension with transparent background and the ""x"" is about 14x14.

6. Does the infobar in Bijou have a light gray background?

7. The text in the Bijou infobar will support Dynamic Type. Please confirm.
 
infobar.png
81.0 KB View Download

Comment 1 by pkl@chromium.org, May 16 2018

1A. Is the light blue color behind the icon the correct shade? Color as shown:
const int kImageBackgroundColor = 0xdce7f6;

Comment 2 by sczs@chromium.org, May 16 2018

Status: Assigned (was: Untriaged)

Comment 3 by pkl@chromium.org, May 17 2018

Blockedon: 817753
Hey Peter. I haven't looked at Khalil's infobar work until today so I've sunk a bit of time getting ramped up. Turns out this is also relevant to a g-pay infobar Mo is working to implement now and has been asking for feedback on. I will post specs that answer your listed questions (and maybe more) tomorrow.

A couple things I can pick off quickly though:

4. Nope. I want to use all the same infobar icons used in Bling today.
5. Attached. It is a template icon and should be tinted the same as the toolbar button icons: black @ 0.5 alpha.
6. No, the background should be the same as used in the tab toolbar (see blocking  issue 817753 ). I will provide more specifics in my next post.
7. I think this is a good place to support dynamic type, but it will make our life harder ... so let me reflect a little more. I will provide more details in my next post.

P.S. Out of curiosity, were you working from specs or just divining meaning from his mocks (if the latter, bravo!)? I want to make sure I'm not missing anything.

Comment 5 by pkl@chromium.org, May 17 2018

"divining meaning"? You are giving me too much credit. I'm practically making s---- up :) I want to get most of the code done and wait for these intrinsic constants from you.
Cc: pschaffner@chromium.org
Owner: pkl@chromium.org
Well you were nearly spot on to what was in his design file, so s---- or not, you deserve credit ;)

=====
SPECS
=====

- Same internal layout metrics (labels, icons, buttons) as today for all permutations.
- (description #1) I think the light blue rectangle that surrounds the infobar icon is too much (sorry to be only saying this now). If it is easy to remove at this point, **please do so**. If not, make it #1A73E8 @ 0.1 alpha with a corner radius of 8.
- (description #2) Primary button background color is #1A73E8 with a corner radius of 8pt.
- (description #3 & 7) Body text **and** button labels should be set at [UIFont preferredFontForTextStyle:UIFontTextStyleBody] save for the primary button (blue background), which should use UIFontTextStyleHeadline. To confirm, this means dynamic type will be supported for infobars, which also means the parent view height should scale appropriately as body text and buttons wrap. There is probably a lot more thought that need to go into defining how to layout everything as type scales, but given our desire to refactor this component's design/code, I think we shouldn't put too much effort into it.
- (description #4) Use the same icons used today for all instances, and assuming they have an alpha channel, treat them as template images and tint them #1A73E8.
- (description #5) The close button asset should be treated as a template image and tinted black @ **0.2** alpha. Said asset (at all resolutions) can be found here: https://drive.google.com/open?id=14E_ea0OpxyHRqKPoHoIcvwKSm0TAjtgy&authuser=pschaffner@google.com
- (description #6) Background should be the same kind of visual effect view used in the toolbar(s) (UIBlurEffectStyleExtraLight with #fafafa @ 0.5 alpha background color on the contentView if I recall correctly, but you should confirm via the currently landed toolbar code.)

---

Nice to haves:
- When there is a bottom toolbar (Compact x Any), the bottom edge of the infobar should sit adjacent the top edge of the [bottom] toolbar. Then, when going fullscreen, the infobar's bottom edge should track the top edge of the toolbar as it scrolls offscreen. (On iPhone X portrait, ~34pts of height must simultaneously be added to the infobar's frame.) If this is too much work, something like what is implemented today [where the infobar sits behind the toolbar] will be acceptable.

---

Assign back to me if I've missed anything!
Project Member

Comment 7 by bugdroid1@chromium.org, May 23 2018

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

commit 04a10147a0479341946266e38b0eb0e89efdb816
Author: Peter K. Lee <pkl@chromium.org>
Date: Wed May 23 07:45:05 2018

Updating Confirm Infobar Styling

- Removed background color on left-icon and added tint
- Changed font-style
- New dismiss icon at top-right
- Changed button corner radius

Screenshot of what the AFTER looks like for iPhone X and iPhone 8:
https://screenshot.googleplex.com/vZw7f4UomSt

Bug:  843571 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I9a0446897298a3d5e51def78de61022e81115c77
Reviewed-on: https://chromium-review.googlesource.com/1068136
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560996}
[modify] https://crrev.com/04a10147a0479341946266e38b0eb0e89efdb816/ios/chrome/app/theme/default_100_percent/infobar_close.png
[modify] https://crrev.com/04a10147a0479341946266e38b0eb0e89efdb816/ios/chrome/app/theme/default_200_percent/infobar_close.png
[modify] https://crrev.com/04a10147a0479341946266e38b0eb0e89efdb816/ios/chrome/app/theme/default_300_percent/infobar_close.png
[modify] https://crrev.com/04a10147a0479341946266e38b0eb0e89efdb816/ios/chrome/browser/ui/infobars/confirm_infobar_view.mm

Looking great! I have been playing with it in canary and I realized I omitted specs for the button highlight states, so here goes:

Close button highlight (pressed) state: assuming this is a UIButton, set its type to UIButtonTypeSystem to get the standard treatment (this is consistent with the toolbar button highlight states: https://bugs.chromium.org/p/chromium/issues/detail?id=807778#c12); if it isn't a UIButton, or if doing so causes too many weird side-effects, setting the view's alpha property to 0.5 should suffice.

As for the action buttons ... I noticed we lost the highlight effect on the non-primary button. I wouldn't mind getting rid of the ink effect (on both buttons) and applying the same solution mentioned above for the close button. I'm also fine keeping the ink effect if it is easier to just reinstate it on the non-primary button. In the end, the important thing here is to *have* a highlight state for all buttons.

Comment 9 by pkl@chromium.org, Jun 1 2018

Status: Fixed (was: Assigned)
The following 3 issues cover the remaining unsolved items here:
   issue 817753 
   issue 848879 
   issue 848880 
This crbug can be closed to reduce clutter.

Sign in to add a comment