core

Monorepo for Tangled tangled.org

1.2k

Fork 127

Add Firefox for Android PWA support #634 #3189

Closed

oyster.cafe 10mo ago

targeting master

Closed opened by and assigned to

oyster.cafe 10mo ago targeting master

This pull request has no versions submitted yet.

Comments

oppi.li 1y ago

the conversion is done quite easily with imagemagick:

convert -density 300 -background none dolly.svg -resize 144x144 dolly-144x144.png

vielle.dev 1y ago

want me to try add it in and replace the in-tree png?

oppi.li 1y ago

yeah that would be nice! if the nix build step could produce PNGs as one of the steps that would be super.

vielle.dev 1y ago

oops wrong area in commit msg 1 sec

vielle.dev 1y ago

large icons are blurry for some reason, heres another fix (sorry for the spam lol)

oppi.li 1y ago

looks pretty good! /dolly.svg is ... not ideal haha. i am going to have to think about what the best way to manage tangled's assets are. we want to be able to style this easily with tailwind, but also avoid turning this into a gohtml template for usecases like this one.

vielle.dev 1y ago

fwiw you could probably make a copy of the template then use sed to remove the template stuff (bc that makes it not valid svg) and then use that temp file and delete it after?

vielle.dev 1y ago

sorry for the resubmit spam (if you get notifications for it) i was having some issues w fixing the merge conflict but i managed to get it sorted !!

oppi.li 1y ago

looking pretty good!

can you format the manifest.json with jq?
we can simplify the nix script to use a loop in bash

#!/bin/bash

sizes=(48 72 96 144 162 512)

for size in "${sizes[@]}"; do
  ${imagemagick}/bin/magick -density 3000 -background none \
    ${src}/appview/pages/templates/fragments/dolly/logo.svg \
    -resize ${size}x${size} \
    $out/pwa-${size}.png
done

oppi.li 1y ago

struggling to get the pkgsStatic variant of appview building, imagemagick is unhappy with the env. its a nixpkgs thing and not an issue with this PR however.

vielle.dev 11mo ago

replaced imagemagick with librsvg and it seems to be working fine now

i really dont know why imagemagick wasnt building but as long as tangled builds lol

oppi.li 11mo ago

thanks for taking a look! i will try this out locally.

oppi.li 11mo ago

couple of thoughts now that i am coming back to this:

we have since introduced OG card drawing libraries (oksvg namely), which is capable of rendering PNGs, we could use that library to use the single-source of truth dolly logo to draw the PWA icons
i think the PWA icon should not be the silhouette, but the full logo IMO. i don't know if there is a degree of customization here (we could serve the silhouette at smaller sizes perhaps)

happy to take over and implement that if you wish to discontinue this, and apologies for the slow review cycle on this!

vielle.dev 11mo ago

wrt oksvg, since afaict its just a go lib without a cli mode would you propose having some go script to generate the svg icons (or something like build.rs if that exists) (or generate them on demand which seems bad lol)
full logo makes sense , just went off what i had for the original PR which was just using the favicon for simplicity. should be fairly trivial to have the icon be the silhouette at smaller sizes but looking at how big the icons would be i dont imagine its a huge issue wrt readability if theyre all the full logo

also no worries about slow review cycle lol, happy to keep chipping away at this one !!

oppi.li 11mo ago

wrt oksvg, since afaict its just a go lib without a cli mode would you propose having some go script to generate the svg icons (or something like build.rs if that exists) (or generate them on demand which seems bad lol)

yes to generate on demand! with the right cache headers this would be negligible overhead on our end (we already do generate a lot of images for OG cards, and these are less cache-friendly mind you!)

vielle.dev 10mo ago

idk where silhouette.svg came from bc i cant find it in the git history but 🤷

the Etag stuff is a little scuffed but it works afaict

the transparent mode and purpose any nonsense is just for the desktop icon since otherwise it would have an ugly box around it which was ugly

id prolly wanna go back and see if i can tidy things up a little and figure out if theres somewhere that works better for this but for now i just wanna make sure this is ~ what you were thinking of

if youre happy with where it is etc im happy to merge rn tho!

vielle.dev 10mo ago

(svg icon was removed as it was causing issues with chrome and shouldnt be required for pwa anyway; will test on firefox/chrome on android later lol)

boltless.me 10mo ago

Sorry for the interrupt. I think generating logo as static file is better here (one from round #8.) While we can do that with proper cache headers, that doesn't mean we should do that if static generation work, I'd prefer that more.

Also, as mentioned from #922, we need actual ico favicon which is easier to pre-generate as static file.

sizes=(48 72 96 144 162 180 512)
for size in "${sizes[@]}"; do
  rsvg-convert \
    -w ${size} -h ${size} \
    -o icon-${size}.png \
    logo.svg

# apple touch icon in 180x180
mv icon-180.png apple-touch-icon.png

icotool -c -o favicon.ico \
  icon-48.png \
  icon-32.png \
  icon-16.png

I can include your work in #922 and coauthor you if you don't mind.

oppi.li 10mo ago

i have read the ico file format spec and it seems trivial to generate via code: https://tangled.org/tangled.org/core/commit/3169e7706e86b963c7e490ec00b428ba36bbb41c. helps us avoid the dependency on icotool.

boltless.me 10mo ago

@oppi.li Can we use it for our own ico builder or are you still thinking of on demand generation?