Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

about:newtab tile favicons are inconsistent in size #11132

Closed
alexwykoff opened this issue Sep 25, 2017 · 4 comments
Closed

about:newtab tile favicons are inconsistent in size #11132

alexwykoff opened this issue Sep 25, 2017 · 4 comments
Labels
bug design A design change, especially one which needs input from the design team. feature/newtab misc/favicon priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). stale

Comments

@alexwykoff
Copy link
Contributor

Description

When visiting the new tab page, the favicons are different sizes.

Steps to Reproduce

Visit a random set of sites, usually Hacker News is ok, but Facebook & YouTube occasionally go through the shrink ray.

  1. Visit a few different sites
  2. Open a new tab
  3. Observe favicons in tiles

Actual result:
screen shot 2017-09-11 at 10 29 55 am

Expected result:
The favicons should all be of a consistent size.

Reproduces how often: [What percentage of the time does it reproduce?]
100%

Brave Version

It's been an issue since newtab was implemented.

about:brave info:

Reproducible on current live release:
Yes

Additional Information

@alexwykoff alexwykoff added bug design A design change, especially one which needs input from the design team. labels Sep 25, 2017
@bsclifton bsclifton added this to the Triage Backlog milestone Nov 27, 2017
@MargarytaChepiga
Copy link
Contributor

MargarytaChepiga commented Apr 9, 2018

Hello! I think this is because we set the img to max-width and max-height. Which sets the limit only for the maximum size of the image. So if the favicon is smaller than 64px it will display it's original weight and height. Here:


The solution I see to this is to make the size static. But then the icons might not look good if their initial size is much smaller. Any ideas? Is anyone is working on this btw?
Before:
before
After:
fix 1_static

@NejcZdovc NejcZdovc added the priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). label Apr 10, 2018
@NejcZdovc NejcZdovc modified the milestones: Triage Backlog, Backlog (Prioritized) Apr 10, 2018
MargarytaChepiga added a commit to MargarytaChepiga/browser-laptop that referenced this issue Apr 11, 2018
@bradleyrichter
Copy link
Contributor

@MargarytaChepiga My only concern with this otherwise nice fix is that, as you said, the 16x size icons will scale up and look fuzzy. Is there a way to add an exception to prevent this?

(only allow favicons to scale up to the desired height/width if they are large enough.)

@MargarytaChepiga
Copy link
Contributor

@bradleyrichter yes, I can do that. But again, the icons won't be the same size.

@MargarytaChepiga
Copy link
Contributor

@bradleyrichter If you mean to leave them the same size ( as in don't scale up 16px) then it's pretty much the same thing as it was before with the only difference that the favicons size is smaller now. Take a look please:
max-32

@bsclifton bsclifton removed this from the Backlog (Prioritized) milestone Aug 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug design A design change, especially one which needs input from the design team. feature/newtab misc/favicon priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). stale
Projects
None yet
Development

No branches or pull requests

5 participants