Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch to a SVG icon + toggling icon status #98

Merged
merged 2 commits into from
Dec 3, 2021
Merged

switch to a SVG icon + toggling icon status #98

merged 2 commits into from
Dec 3, 2021

Conversation

brunob
Copy link
Owner

@brunob brunob commented Dec 1, 2021

use icon from https://github.com/Leaflet/Leaflet.fullscreen optimised with https://jakearchibald.github.io/svgomg/

ref #97 & #87

Maybe we should use icons from https://remixicon.com/ but for a first shot it seems good to me like this...

Any thoughts @hswong3i & @skirridsystems ?

@skirridsystems
Copy link
Contributor

Works for me. I always liked the icon used by Leaflet.fullscreen.

@hswong3i
Copy link

hswong3i commented Dec 2, 2021

With #97 research progress, I would like to suggest NOT using Leaflet.fullscreen legacy SVG icon (because their CSS also not using it, but the exported PNG version).

Moreover, I vote for Font Awesome https://fontawesome.com/v5.15/icons/expand and https://fontawesome.com/v5.15/icons/compress, looks much in pair style.

@brunob
Copy link
Owner Author

brunob commented Dec 2, 2021

Thx for the feedback, @hswong3i i've compressed the SVG from Leaflet.fullscreen 5.89 KB => 442 Bytes

I also like Font Awesome alternative (which are similar to the shape proposed by https://remixicon.com/ fullscreen icons), but i also think that's a good thing to use the same icon than Leaflet.fullscreen :\

@brunob
Copy link
Owner Author

brunob commented Dec 2, 2021

Here is an alternative icon using fontawesome if you want to give it a try. It feel better on eyes cause the stroke seems a bit bolder, but it's also a bigger file 945 bytes (compared to the 442 bytes of the one in this PR).

<svg viewBox="0 0 26 52" xmlns="http://www.w3.org/2000/svg"><path d="M20.6 36.7H16a.9.9 0 0 1-.8-.8v-4.5c0-.2.2-.4.4-.4h1.4c.3 0 .5.2.5.4v3h3c.2 0 .4.2.4.5v1.4c0 .2-.2.4-.4.4zm-9.9-.8v-4.5c0-.2-.2-.4-.4-.4H8.9c-.3 0-.5.2-.5.4v3h-3c-.2 0-.4.2-.4.5v1.4c0 .2.2.4.4.4H10c.4 0 .8-.4.8-.8zm0 10.7V42c0-.4-.4-.8-.8-.8H5.4c-.2 0-.4.2-.4.4v1.4c0 .3.2.5.4.5h3v3c0 .2.2.4.5.4h1.4c.2 0 .4-.2.4-.4zm6.9 0v-3h3c.2 0 .4-.2.4-.5v-1.4c0-.2-.2-.4-.4-.4H16c-.4 0-.8.4-.8.8v4.5c0 .2.2.4.4.4h1.4c.3 0 .5-.2.5-.4zM5 10.3V5.9c0-.5.4-.9.9-.9h4.4c.2 0 .4.2.4.4V7c0 .2-.2.4-.4.4h-3v3c0 .2-.2.4-.4.4H5.4a.4.4 0 0 1-.4-.4zm10.3-4.9V7c0 .2.2.4.4.4h3v3c0 .2.2.4.4.4h1.5c.2 0 .4-.2.4-.4V5.9c0-.5-.4-.9-.9-.9h-4.4c-.2 0-.4.2-.4.4zm5.3 9.9H19c-.2 0-.4.2-.4.4v3h-3c-.2 0-.4.2-.4.4v1.5c0 .2.2.4.4.4h4.4c.5 0 .9-.4.9-.9v-4.4c0-.2-.2-.4-.4-.4zm-9.9 5.3V19c0-.2-.2-.4-.4-.4h-3v-3c0-.2-.2-.4-.4-.4H5.4c-.2 0-.4.2-.4.4v4.4c0 .5.4.9.9.9h4.4c.2 0 .4-.2.4-.4z" fill="currentColor"/></svg>

You can test it on https://github.com/brunob/leaflet.fullscreen/tree/svg_icon_fa branch

@brunob brunob merged commit cf5c2ac into master Dec 3, 2021
@brunob brunob deleted the svg_icon branch December 3, 2021 12:47
@brunob brunob mentioned this pull request Dec 3, 2021
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