-
Notifications
You must be signed in to change notification settings - Fork 973
convert URL bar FA icons to sprite PNG (or SVG) #5891
Comments
CC @bbondy |
nice work! |
Starting to implement these using Aphrodite! So far, so good 😄 |
@bradleyrichter I made good progress with this last nite (which you can see here: https://github.com/bsclifton/browser-laptop/commit/9091b3170d4906bc8a134108d26d2d73003656ae), but I was not able to finish it unfortunately. Would you be OK if we bumped this to 0.13.1? None of the urlbar icons have changed or gotten worse between 0.12.10 and now |
I thought they should be SVG instead of PNG? @bradleyrichter |
@luixxiul SVGs have no margin and therefore can not be visually aligned in a grid line. (unless there is a way that I have not seen yet) PNGs allow us to simplify the code and use centering. If a bump is needed, it's done in the image file. |
So will the PNGs still work with high-resolution displays and accessibility? |
@bradleyrichter I shared some notes on Slack; I'd vote to stick with PNG for now but we should proof of concept something (might be a good example on about:styles) |
I use svg at work and they way better in terms of responsive design, scalibility, etc. Based on what i have seen in the code, I think we have over polluted our css with alignment correction thats why we are facing issue with icon alignments. Example with issue: https://github.com/brave/browser-laptop/blob/master/img/bitcoin_color.svg Suggestion: We should stick with svg and update the svg files to be consistent on our standrd canvas size and the icon should occupy the whole canvas. With that we can remove a lot of css hacks for positioning. |
@gyandeeps If you were up for helping with regards to SVG, we could definitely use your help 😄 I'll finish this particular issue using sprites (since it's almost done already)... but you can reach out to @bradleyrichter (over Slack might be easiest) to get a copy of the above as an SVG sprite. We could use your help understanding if it looks OK or what could be better 😄 |
@bsclifton I can help to get alignment work done for sure. Since i am no expert in creating svg files that's where we would need a person who has that expertise. |
I don't think this will make the cut for 0.13.2; moving back to 0.13.3 for now |
@gyandeeps you did a great job with the preferences page (captured in #6904). Would you like to give this one a go? I had completed this a while back but with PNG (link shown above). If not, it's OK- let me know 😄 |
I am ok with working on this. But i think we should do the aphrodite work first if needed and then do svg work. I just dont want to mix the 2 and complicate the stuff. If that work is already done then i can work on this. |
@gyandeeps OK that sounds good- I'll keep self assigned for now and look at converting to Aphrodite (which I had done above, just needs a rebase). At that point (as long as the SVGs are the correct size, etc), it should be plug-n-play 😄 |
Moving back to post 1.0 @bradleyrichter when you have a chance, you can attach the images in SVG format (now that we understand how to do it properly). Then I'll be able to knock this out 😄 |
@petemill closed out most of these issues with recent style update 😄 |
Using a sprite grid for our URL icons will allow us to control the alignment of each icon more easily within the image file without the need for unreliable pixel bumping in CSS.
20 icon positions are available initially.
Each icon is centered on the grid.
A1 - unused (future verified certificate)
A2 - HTTPS (secure)
A3 - unused (future mixed HTTPS)
A4 - HTTP (non-secure)
A5 - unused (normal file)
A6 - search (normal)
A7 - unused (info)
A8 - alert page (all error results pages)
A9 - brave payments = yes
A10 - all brave about:pages
B1 - verified tag (used in brave payment table)
B2 - brave hosted page
B3 - unused (brave alert)
B4 - HTTP for title mode (non-secure)
B5 - unused (normal file - grey)
B6 - unused (search - grey)
B7 - unused (info - grey)
B8 - noscript active
B9 - brave payments = no
B10 - unused (all brave about:pages - grey)
The text was updated successfully, but these errors were encountered: