Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#2050 closed change (fixed)

Bubble popup shouldn't be reusing the logo from the first-run page

Reported by: trev Assignee: trev
Priority: P1 Milestone: Adblock-Plus-1.8.12-for-Chrome-Opera-Safari
Module: User-Interface Keywords:
Cc: sven, sebastian, greiner Blocked By:
Blocking: #1533 Platform: Unknown
Ready: yes Confidential: no
Tester: Verified working:
Review URL(s):

http://codereview.adblockplus.org/5706591068225536/

Description (last modified by greiner)

Background

The bubble popup is currently scaling down the logo we use on the first-run page (99x100 to 55x55). We should really be using a logo with the right size here.

What to change

Use the existing 64x64 icon64.png image as its size most closely matches the one used right now.

Attachments (2)

old.png (27.1 KB) - added by trev 5 years ago.
new.png (27.9 KB) - added by trev 5 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 5 years ago by trev

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 5 years ago by trev

  • Cc greiner added
  • Component changed from Platform to User-Interface

Changed 5 years ago by trev

Changed 5 years ago by trev

comment:3 Changed 5 years ago by trev

What this currently looks like (scaled down by factor 2 because this is a Retina Mac):

Old New

comment:4 follow-up: Changed 5 years ago by sebastian

How about <img src="abp-64.png" srcset="abp-128.png 2x"> for full DPI? ;)

comment:5 in reply to: ↑ 4 Changed 5 years ago by greiner

  • Description modified (diff)
  • Ready set

Replying to sebastian:

How about <img src="abp-64.png" srcset="abp-128.png 2x"> for full DPI? ;)

It might make more sense to do that in a separate issue since none of the other images in the popup are optimized for high-DPI screens at this point so they'd need to be changed as well. Also note that abp-64.png is called icon64.png and that there is no abp-128.png in the "adblockplus" repository so that would need to be added first.

comment:6 Changed 5 years ago by sebastian

Well, this change is still in the adblockpluschrome repo. But yeah, I'm fine with introducing srcset with a separate issue. It just came to my mind when I read "Retina". ;)

comment:7 Changed 5 years ago by trev

Actually, that's the one image in the popup where using a high-resolution image makes a huge difference - and it's absolutely trivial. So I implemented it.

comment:8 Changed 5 years ago by greiner

  • Priority changed from Unknown to P1

comment:9 Changed 5 years ago by trev

  • Priority changed from P1 to Unknown
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:10 Changed 5 years ago by trev

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
  • Priority changed from Unknown to P1

comment:11 Changed 5 years ago by trev

  • Blocking 1533 added
Note: See TracTickets for help on using tickets.