brunob / leaflet.fullscreen

Leaflet.Control.FullScreen for Leaflet
https://brunob.github.io/leaflet.fullscreen/
MIT License
376 stars 107 forks source link

Rework PNG Icon with SVG and SCSS #97

Closed hswong3i closed 2 years ago

hswong3i commented 2 years ago

Existing implementation using icon-fullscreen.png and icon-fullscreen-2x.png for different screen (e.g. iOS with retina display). Moreover, we don't have any toggle effect for the icon status.

This PR rework the icon with:

Some style reuse from: https://github.com/domoritz/leaflet-locatecontrol/pull/296

Online demo: https://drustack.github.io/brunob-leaflet.fullscreen/

Fix #87

Signed-off-by: Wong Hoi Sing Edison hswong3i@pantarei-design.com

brunob commented 2 years ago

Thx for the PR, really appreciate :) but i think this is an oversized code change for a small functional change. I'd like to :

Anyway, i think the icon change on toggle is really needed change + the switch to a SVG icon (maybe we should use the one from https://github.com/Leaflet/Leaflet.fullscreen/blob/gh-pages/fullscreen.svg)

hswong3i commented 2 years ago

My similar PR: https://github.com/Leaflet/Leaflet.fullscreen/pull/118

My initial though are:

BTW, I also face a critical issue here:

Sorry that I will shift my focus back to using https://github.com/Leaflet/Leaflet.fullscreen (even it is inactive, I could using my patched version for my client project, now).