FIX: Refactor lightbox mobile icon

FIX: Refactor lightbox mobile icon
  • Fixes a performance issue on a site with lots of images in posts

  • turns out that “filter: invert(100%)” performs very poorly on Safari/iPhone

  • also disables transition on the element on mobile

From 06d1b19ca2d8994d444ebfa3bb17634a4ae5ea7b Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Thu, 6 Dec 2018 15:32:26 -0500
Subject: [PATCH] FIX: Refactor lightbox mobile icon

- Fixes a performance issue on a site with lots of images in posts

- turns out that "filter: invert(100%)" performs very poorly on Safari/iPhone

- also disables transition on the element on mobile

diff --git a/app/assets/stylesheets/mobile/lightbox.scss b/app/assets/stylesheets/mobile/lightbox.scss
index d1ca00b..5f3d98b 100644
--- a/app/assets/stylesheets/mobile/lightbox.scss
+++ b/app/assets/stylesheets/mobile/lightbox.scss
@@ -1,6 +1,7 @@
 .lightbox .meta,
 .lightbox:hover .meta {
   opacity: 0.7;
+  transition: none;
 }
 
 .meta {
@@ -21,8 +22,15 @@
 
   .expand {
     position: initial;
-    filter: invert(100%);
     float: none;
     height: 16px;
+    &:before {
+      // ideally, the SVG used here should be in HTML and reference the SVG sprite
+      // the SVG used here is the "expand" icon from FontAwesome 4.7.0
+      content: svg-uri(
+        '<svg xmlns="http://www.w3.org/2000/svg" width="14px" height="16px" viewBox="0 0 1792 1792" fill="#{$primary-high}"><path d="M883 1056q0 13-10 23l-332 332 144 144q19 19 19 45t-19 45-45 19h-448q-26 0-45-19t-19-45v-448q0-26 19-45t45-19 45 19l144 144 332-332q10-10 23-10t23 10l114 114q10 10 10 23zm781-864v448q0 26-19 45t-45 19-45-19l-144-144-332 332q-10 10-23 10t-23-10l-114-114q-10-10-10-23t10-23l332-332-144-144q-19-19-19-45t19-45 45-19h448q26 0 45 19t19 45z"/></svg>'
+      );
+      opacity: inherit;
+    }
   }
 }

GitHub

This feels super hacky to me, why don’t we just ship an extra “inverse” icon here in the discourse part of the svg bundle?

I worry about introducing SVG glyphs into our CSS directly.

2 Likes

The thing is, none of the icons in the lightbox are in the SVG bundle. I opted for the svg-uri workaround here because adding SVG icons inline would require a post rebake.

Is it trivial to mark all posts with images as needing a rebake? If so, I can refactor this to remove the SVG in CSS almost everywhere (the only other places with SVGs in CSS would be oneboxes, I think.)

It is actually surprisingly simple, all you need to do is run a query that sets baked_version to NULL on all posts where you see lightbox in cooked then the regular job will pick it up.

Then over a few days this will trickle in.

I think this is a reasonable solution here even if you just apply the change going forward, do the migration and add a #TODO to the CSS to say “delete me in Feb 2019”

1 Like

Ok, that’s perfect. I will set that up in the next few days.

2 Likes

Did we end up sorting this out?

2 Likes

I did not, until now: DEV: Refactor icons used in lightbox HTML

1 Like