Skip to content

Iconsets#105

Closed
tophf wants to merge 6 commits into
masterfrom
iconsets
Closed

Iconsets#105
tophf wants to merge 6 commits into
masterfrom
iconsets

Conversation

@tophf

@tophf tophf commented Jun 28, 2017

Copy link
Copy Markdown
Member

Related: #7.

  1. Currently only 16px and 32px icons are of high quality.
  2. Waiting for someone to make high quality 38px icons.
  3. favicon on own pages is changed accordingly only in Firefox because it doesn't display it initially so we have to display something ourselves. Chrome is too smart & dumb: it automatically selects the best resolution for our default icon in manifest.json but not if we add the favicon like we do in Firefox: it'll use only one hard-coded size which will look blurry if the user has different OS window metrics.

clip295-fs8

@tophf tophf force-pushed the iconsets branch 2 times, most recently from 2477aaa to 32270d4 Compare June 28, 2017 13:35
@narcolepticinsomniac

Copy link
Copy Markdown
Member

Very cool. Wish I hadn't dropped the ball on accounting for all necessary sizes, but we'll get em done.

@tophf tophf force-pushed the iconsets branch 3 times, most recently from c0215a0 to 79e06ad Compare June 29, 2017 15:46
@Mottie

Mottie commented Jul 2, 2017

Copy link
Copy Markdown
Member

Wouldn't an svg be easier to use and style?

I put together this svg that is 1024x1024: https://gist.github.com/Mottie/f01aa09471ea39c2b561c1b2b2e22fd9

You can change the style by targeting the .border-s (border+S color) and .background class names:

svg .background {
  fill: #28fefe;
}
svg .border-s {
  fill: #285959;
}

I don't know if I got the font of the "S" exactly right, so I tweaked it manually.

@tophf

tophf commented Jul 2, 2017

Copy link
Copy Markdown
Member Author

@Mottie, see https://crbug.com/29683: SVG is not yet supported for the extension icon, which is what the feature is about. And even if it was supported, the automatic scaling provides subpar results for 16px size.

@Mottie

Mottie commented Jul 2, 2017

Copy link
Copy Markdown
Member

Oh ok...

Well, is there anything you need help working on? I was added to the organization, so I feel like I need to do something other than make noise

@tophf

tophf commented Jul 2, 2017

Copy link
Copy Markdown
Member Author

Well, there are many things to do. I mostly work on bug fixes and trivial features.

@narcolepticinsomniac

narcolepticinsomniac commented Jul 4, 2017

Copy link
Copy Markdown
Member

I mentioned in #7 that separated layers for 38px are missing, which I really kinda need to do them because scaling anything tends to turn into a mess. Anyway, I did the 32px sets. @tophf You have a high res that uses them, correct? I can't really easily test them in the browser, but GIMP shows a scaled version in the header (14px I think), and they LGTM. You'll be able to tell better if you can see them in action.

I used the exact shades to colorize layers, but the resulting hexes deviated by a minuscule amount. They should be indistinguishable. I doubled the borders since they're twice the size, and didn't look right without doing so. I also fixed a bunch of individual pixel irregularities for each. Check em out, and we'll take it from there. If they're good, I pretty much have the process down, so the 38px should be easier once I have the separated layers. Hopefully @d3n1c1d3 (or any other volunteer) can hook them up.

32 Sets.zip

@tophf

tophf commented Jul 4, 2017

Copy link
Copy Markdown
Member Author

Thanks, added. There was an almost transparent color in the first column so I've deleted it. You could see it an extreme zoom against a white bg.

You have a high res that uses them

It's single-density 30", not retina. I think the icons look fine though.

@tophf

tophf commented Jul 8, 2017

Copy link
Copy Markdown
Member Author

The new 38px icons are fine.
I guess this PR can be merged, right?

@tophf tophf closed this Jul 9, 2017
@tophf tophf deleted the iconsets branch July 9, 2017 09:23
@tophf

tophf commented Jul 9, 2017

Copy link
Copy Markdown
Member Author

Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants