Merge pull request #189 from ably/fix-for-multiple-logos-rendering-in…

…correctly Fix for multiple logos rendering incorrectly
ably · May 31, 2022 · dfd3eb6 · dfd3eb6
2 parents cfd081a + 041279c
commit dfd3eb6
Show file tree

Hide file tree

Showing 17 changed files with 111 additions and 64 deletions.
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    ably-ui (7.8.0)
+    ably-ui (7.8.0.dev.502e5bf)
       view_component (>= 2.33, < 2.50)
 
 GEM

diff --git a/lib/ably_ui/version.rb b/lib/ably_ui/version.rb
@@ -1,3 +1,3 @@
 module AblyUi
-  VERSION = '7.8.0'
+  VERSION = '7.8.0.dev.502e5bf'
 end
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@ably/ui",
-  "version": "7.8.0",
+  "version": "7.8.0-dev.502e5bf",
   "description": "Home of the Ably design system library ([design.ably.com](https://design.ably.com)). It provides a showcase, development/test environment and a publishing pipeline for different distributables.",
   "repository": {
     "type": "git",

diff --git a/preview/Gemfile b/preview/Gemfile
@@ -36,4 +36,4 @@ gem 'view_component', '~> 2.33.0', require: 'view_component/engine'
 
 gem 'responders'
 
-gem 'ably-ui', '7.8.0', require: 'ably_ui'
+gem 'ably-ui', '7.8.0.dev.502e5bf', require: 'ably_ui'
diff --git a/preview/Gemfile.lock b/preview/Gemfile.lock
@@ -1,7 +1,7 @@
 GEM
   remote: https://rubygems.org/
   specs:
-    ably-ui (7.8.0)
+    ably-ui (7.8.0.dev.502e5bf)
       view_component (>= 2.33, < 2.50)
     actioncable (6.0.3.4)
       actionpack (= 6.0.3.4)
@@ -169,7 +169,7 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
-  ably-ui (= 7.8.0)
+  ably-ui (= 7.8.0.dev.502e5bf)
   bootsnap (>= 1.4.2)
   byebug
   listen (~> 3.2)

diff --git a/preview/app/javascript/packs/application.js b/preview/app/javascript/packs/application.js
@@ -11,6 +11,7 @@ import Icon from "@ably/ui/core/Icon";
 import FeaturedLink from "@ably/ui/core/FeaturedLink";
 import CustomerLogos from "@ably/ui/core/CustomerLogos";
 import Loader from "@ably/ui/core/Loader";
+import Logo from "@ably/ui/core/Logo";
 
 import {
   reactRenderer,
@@ -51,7 +52,8 @@ document.addEventListener("DOMContentLoaded", () => {
     Icon,
     FeaturedLink,
     CustomerLogos,
-    Loader
+    Loader,
+    Logo
   });
 
   // Fetch additional data, trigger a re-render for components subscribed to store

diff --git a/preview/app/views/components/logo_params.yml b/preview/app/views/components/logo_params.yml
@@ -0,0 +1,11 @@
+---
+- :name: href
+  :optional: "Yes"
+  :default: "/"
+  :type: String
+  :description: Target URL
+- :name: dataId
+  :optional: "Yes"
+  :default: ""
+  :type: String
+  :description: Identifier for DOM manipulation
diff --git a/preview/app/views/components/logo_react.html.erb b/preview/app/views/components/logo_react.html.erb
@@ -0,0 +1,13 @@
+<%= render(TitleComponent.new(react: true, vw: true)) do %>
+  Logo
+<% end %>
+
+ <%= render(ParameterTableComponent.new(component_parameters, framework)) %>
+
+<%= react_component('logo') %>
+
+<% content_for :component do %>
+  <script type="text/javascript">
+    document.body.classList.add("bg-light-grey", "m-32");
+  </script>
+<% end %>
diff --git a/preview/app/views/components/logo_vw.html.erb b/preview/app/views/components/logo_vw.html.erb
@@ -0,0 +1,13 @@
+<%= render(TitleComponent.new(react: true, vw: true)) do %>
+  Logo
+<% end %>
+
+<%= render(ParameterTableComponent.new(component_parameters, framework)) %>
+
+<%= render(AblyUi::Core::Logo.new(href: '/')) %>
+
+<% content_for :component do %>
+  <script type="text/javascript">
+    document.body.classList.add("bg-light-grey", "m-32");
+  </script>
+<% end %>
diff --git a/preview/app/views/pages/index.html.erb b/preview/app/views/pages/index.html.erb
@@ -7,6 +7,9 @@
         <li>
           <p class="font-medium">Components</p>
           <ul class="pl-16">
+            <li>
+              <a href="/components/logo">Logo</a>
+            </li>
             <li>
               <a href="/components/meganav">Meganav</a>
             </li>

diff --git a/preview/package.json b/preview/package.json
@@ -2,7 +2,7 @@
   "name": "preview",
   "private": true,
   "dependencies": {
-    "@ably/ui": "7.8.0",
+    "@ably/ui": "7.8.0-dev.502e5bf",
     "@babel/preset-react": "^7.12.5",
     "@rails/ujs": "^6.0.0",
     "@rails/webpacker": "4.3.0",

diff --git a/preview/yarn.lock b/preview/yarn.lock
@@ -2,10 +2,10 @@
 # yarn lockfile v1
 
 
-"@ably/[email protected]":
-  version "7.8.0"
-  resolved "https://registry.yarnpkg.com/@ably/ui/-/ui-7.8.0.tgz#c7134f4aaae86c2e649571afa056cad33a165fcf"
-  integrity sha512-E500PPjKwcOGLVvUtv7SF1gucpketFk1ob+vYsrLFf2jhDf+3YSSxQlkZkUCFriMClnCzQ/AByRcRgQY6EnUlg==
+"@ably/[email protected]-dev.502e5bf":
+  version "7.8.0-dev.502e5bf"
+  resolved "https://registry.yarnpkg.com/@ably/ui/-/ui-7.8.0-dev.502e5bf.tgz#7a8470a0b2bd612daf98e971c449a809510eabf1"
+  integrity sha512-0uHl42+DwSLcozsMywpSkz8I2lsbvlbUAfGIOYajBaNF6ZBGZjTCx1GyAWNIV27OTuhwJIJUm0za2W+/jAL2ug==
   dependencies:
     array-flat-polyfill "^1.0.1"
     deepmerge "^4.2.2"

diff --git a/src/core/Logo/component.html.erb b/src/core/Logo/component.html.erb
@@ -1,21 +1,21 @@
 <%= link_to logo_href, class: 'h-32' do %>
-  <%= tag.svg class: ["transition-colors", theme(:logo_text_color)], data: { id: data_id }, width: "108", height: "32", viewBox: "0 0 108 32", xmlns: "http://www.w3.org/2000/svg" do %>
+  <%= tag.svg class: ["transition-colors", "text-cool-black"], data: { id: data_id }, width: "108", height: "32", viewBox: "0 0 108 32", xmlns: "http://www.w3.org/2000/svg" do %>
     <path
       d="M62.922 24.9786V4.08813H66.6933V11.6512C67.9709 10.435 69.6164 9.76044 71.3538 9.76044C75.4318 9.76044 79.0498 12.8674 79.0498 17.5484C79.0498 22.2293 75.4318 25.3465 71.3538 25.3465C69.5244 25.3465 67.7971 24.6209 66.5094 23.3024V24.9786H62.922ZM75.2785 17.5484C75.2785 14.932 73.4183 13.1025 70.9859 13.1025C68.6148 13.1025 66.7853 14.84 66.6933 17.3644V17.5484C66.6933 20.1648 68.5534 21.9942 70.9859 21.9942C73.4183 21.9942 75.2785 20.1648 75.2785 17.5484ZM80.7975 24.9786V4.08813H84.5688V24.9786H80.7975ZM89.8425 30.3954L92.0399 25.1523L86.0712 10.1284H90.1491L93.9511 20.6247L97.8144 10.1284H101.954L93.8591 30.4056H89.8425V30.3954ZM56.9329 10.1284V12.0191C55.6247 10.5883 53.7952 9.77066 51.9147 9.77066C47.8367 9.77066 44.2187 12.8777 44.2187 17.5586C44.2187 22.2497 47.8367 25.3465 51.9147 25.3465C53.8668 25.3465 55.7166 24.4982 57.0555 22.9754V24.9888H60.3465V10.1284H56.9329ZM56.5649 17.5484C56.5649 20.1341 54.7048 21.9942 52.2724 21.9942C49.8399 21.9942 47.9798 20.1341 47.9798 17.5484C47.9798 14.9626 49.8399 13.1025 52.2724 13.1025C54.6435 13.1025 56.473 14.8706 56.5649 17.3644V17.5484Z"
       fill="currentColor"
     />
-    <path d="M19.2858 0L3.14788 29.5369L0 27.3293L14.932 0H19.2858ZM19.5107 0L35.6487 29.5369L38.7965 27.3293L23.8646 0H19.5107Z" fill="url(#paint0_linear)" />
-    <path d="M35.4238 29.7107L19.3983 17.16L3.37271 29.7107L6.64323 32L19.3983 22.0147L32.1533 32L35.4238 29.7107Z" fill="url(#paint1_linear)" />
+    <path d="M19.2858 0L3.14788 29.5369L0 27.3293L14.932 0H19.2858ZM19.5107 0L35.6487 29.5369L38.7965 27.3293L23.8646 0H19.5107Z" fill="url(#<%= gradient_id_0 %>)" />
+    <path d="M35.4238 29.7107L19.3983 17.16L3.37271 29.7107L6.64323 32L19.3983 22.0147L32.1533 32L35.4238 29.7107Z" fill="url(#<%= gradient_id_1 %>)" />
     <defs>
-      <linearGradient id="paint0_linear" x1="5.47361" y1="37.4219" x2="32.4603" y2="7.45023" gradientUnits="userSpaceOnUse">
+      <linearGradient id="<%= gradient_id_0 %>" x1="5.47361" y1="37.4219" x2="32.4603" y2="7.45023" gradientUnits="userSpaceOnUse">
         <stop stop-color="#FF5416" />
         <stop offset="0.2535" stop-color="#FF5115" />
         <stop offset="0.461" stop-color="#FF4712" />
         <stop offset="0.6523" stop-color="#FF350E" />
         <stop offset="0.8327" stop-color="#FF1E08" />
         <stop offset="1" stop-color="#FF0000" />
       </linearGradient>
-      <linearGradient id="paint1_linear" x1="10.7084" y1="39.3593" x2="26.6583" y2="21.6452" gradientUnits="userSpaceOnUse">
+      <linearGradient id="<%= gradient_id_1 %>" x1="10.7084" y1="39.3593" x2="26.6583" y2="21.6452" gradientUnits="userSpaceOnUse">
         <stop stop-color="#FF5416" />
         <stop offset="0.2535" stop-color="#FF5115" />
         <stop offset="0.461" stop-color="#FF4712" />

diff --git a/src/core/Logo/component.jsx b/src/core/Logo/component.jsx
@@ -1,50 +1,52 @@
-import React from "react";
+import React, { useMemo } from "react";
 import T from "prop-types";
 
-const Logo = ({ theme, dataId, href = "/" }) => (
-  <a href={href} className="h-32">
-    <svg
-      data-id={dataId}
-      className={`${theme.logoTextColor} transition-colors`}
-      width="108"
-      height="32"
-      viewBox="0 0 108 32"
-      xmlns="http://www.w3.org/2000/svg"
-    >
-      <path
-        d="M62.922 24.9786V4.08813H66.6933V11.6512C67.9709 10.435 69.6164 9.76044 71.3538 9.76044C75.4318 9.76044 79.0498 12.8674 79.0498 17.5484C79.0498 22.2293 75.4318 25.3465 71.3538 25.3465C69.5244 25.3465 67.7971 24.6209 66.5094 23.3024V24.9786H62.922ZM75.2785 17.5484C75.2785 14.932 73.4183 13.1025 70.9859 13.1025C68.6148 13.1025 66.7853 14.84 66.6933 17.3644V17.5484C66.6933 20.1648 68.5534 21.9942 70.9859 21.9942C73.4183 21.9942 75.2785 20.1648 75.2785 17.5484ZM80.7975 24.9786V4.08813H84.5688V24.9786H80.7975ZM89.8425 30.3954L92.0399 25.1523L86.0712 10.1284H90.1491L93.9511 20.6247L97.8144 10.1284H101.954L93.8591 30.4056H89.8425V30.3954ZM56.9329 10.1284V12.0191C55.6247 10.5883 53.7952 9.77066 51.9147 9.77066C47.8367 9.77066 44.2187 12.8777 44.2187 17.5586C44.2187 22.2497 47.8367 25.3465 51.9147 25.3465C53.8668 25.3465 55.7166 24.4982 57.0555 22.9754V24.9888H60.3465V10.1284H56.9329ZM56.5649 17.5484C56.5649 20.1341 54.7048 21.9942 52.2724 21.9942C49.8399 21.9942 47.9798 20.1341 47.9798 17.5484C47.9798 14.9626 49.8399 13.1025 52.2724 13.1025C54.6435 13.1025 56.473 14.8706 56.5649 17.3644V17.5484Z"
-        fill="currentColor"
-      />
-      <path
-        d="M19.2858 0L3.14788 29.5369L0 27.3293L14.932 0H19.2858ZM19.5107 0L35.6487 29.5369L38.7965 27.3293L23.8646 0H19.5107Z"
-        fill="url(#paint0_linear)"
-      />
-      <path d="M35.4238 29.7107L19.3983 17.16L3.37271 29.7107L6.64323 32L19.3983 22.0147L32.1533 32L35.4238 29.7107Z" fill="url(#paint1_linear)" />
-      <defs>
-        <linearGradient id="paint0_linear" x1="5.47361" y1="37.4219" x2="32.4603" y2="7.45023" gradientUnits="userSpaceOnUse">
-          <stop stopColor="#FF5416" />
-          <stop offset="0.2535" stopColor="#FF5115" />
-          <stop offset="0.461" stopColor="#FF4712" />
-          <stop offset="0.6523" stopColor="#FF350E" />
-          <stop offset="0.8327" stopColor="#FF1E08" />
-          <stop offset="1" stopColor="#FF0000" />
-        </linearGradient>
-        <linearGradient id="paint1_linear" x1="10.7084" y1="39.3593" x2="26.6583" y2="21.6452" gradientUnits="userSpaceOnUse">
-          <stop stopColor="#FF5416" />
-          <stop offset="0.2535" stopColor="#FF5115" />
-          <stop offset="0.461" stopColor="#FF4712" />
-          <stop offset="0.6523" stopColor="#FF350E" />
-          <stop offset="0.8327" stopColor="#FF1E08" />
-          <stop offset="1" stopColor="#FF0000" />
-        </linearGradient>
-      </defs>
-    </svg>
-  </a>
-);
+const Logo = ({ dataId, href = "/" }) => {
+  // This fixes a bug where if the logo is rendered more than once on the page, and one of the instances
+  // if it is hidden, the other instance will not show the ably shape from the logo.
+
+  // This is because the defs in this SVG reference ids that need to be unique ids. The browser discards the "newer"
+  // linearGradients defined in the other logo, and inherits the `hidden` style from the first.
+  const createID = () => `paint_gradient_${Date.now()}`;
+  const gradientIds = useMemo(() => [createID(), createID()], []);
+
+  return (
+    <a href={href} className="h-32">
+      <svg data-id={dataId} className="text-cool-black transition-colors" width="108" height="32" viewBox="0 0 108 32" xmlns="http://www.w3.org/2000/svg">
+        <path
+          d="M62.922 24.9786V4.08813H66.6933V11.6512C67.9709 10.435 69.6164 9.76044 71.3538 9.76044C75.4318 9.76044 79.0498 12.8674 79.0498 17.5484C79.0498 22.2293 75.4318 25.3465 71.3538 25.3465C69.5244 25.3465 67.7971 24.6209 66.5094 23.3024V24.9786H62.922ZM75.2785 17.5484C75.2785 14.932 73.4183 13.1025 70.9859 13.1025C68.6148 13.1025 66.7853 14.84 66.6933 17.3644V17.5484C66.6933 20.1648 68.5534 21.9942 70.9859 21.9942C73.4183 21.9942 75.2785 20.1648 75.2785 17.5484ZM80.7975 24.9786V4.08813H84.5688V24.9786H80.7975ZM89.8425 30.3954L92.0399 25.1523L86.0712 10.1284H90.1491L93.9511 20.6247L97.8144 10.1284H101.954L93.8591 30.4056H89.8425V30.3954ZM56.9329 10.1284V12.0191C55.6247 10.5883 53.7952 9.77066 51.9147 9.77066C47.8367 9.77066 44.2187 12.8777 44.2187 17.5586C44.2187 22.2497 47.8367 25.3465 51.9147 25.3465C53.8668 25.3465 55.7166 24.4982 57.0555 22.9754V24.9888H60.3465V10.1284H56.9329ZM56.5649 17.5484C56.5649 20.1341 54.7048 21.9942 52.2724 21.9942C49.8399 21.9942 47.9798 20.1341 47.9798 17.5484C47.9798 14.9626 49.8399 13.1025 52.2724 13.1025C54.6435 13.1025 56.473 14.8706 56.5649 17.3644V17.5484Z"
+          fill="currentColor"
+        />
+        <path
+          d="M19.2858 0L3.14788 29.5369L0 27.3293L14.932 0H19.2858ZM19.5107 0L35.6487 29.5369L38.7965 27.3293L23.8646 0H19.5107Z"
+          fill={`url(#${gradientIds[0]})`}
+        />
+        <path d="M35.4238 29.7107L19.3983 17.16L3.37271 29.7107L6.64323 32L19.3983 22.0147L32.1533 32L35.4238 29.7107Z" fill={`url(#${gradientIds[1]})`} />
+        <defs>
+          <linearGradient id={gradientIds[0]} x1="5.47361" y1="37.4219" x2="32.4603" y2="7.45023" gradientUnits="userSpaceOnUse">
+            <stop stopColor="#FF5416" />
+            <stop offset="0.2535" stopColor="#FF5115" />
+            <stop offset="0.461" stopColor="#FF4712" />
+            <stop offset="0.6523" stopColor="#FF350E" />
+            <stop offset="0.8327" stopColor="#FF1E08" />
+            <stop offset="1" stopColor="#FF0000" />
+          </linearGradient>
+          <linearGradient id={gradientIds[1]} x1="10.7084" y1="39.3593" x2="26.6583" y2="21.6452" gradientUnits="userSpaceOnUse">
+            <stop stopColor="#FF5416" />
+            <stop offset="0.2535" stopColor="#FF5115" />
+            <stop offset="0.461" stopColor="#FF4712" />
+            <stop offset="0.6523" stopColor="#FF350E" />
+            <stop offset="0.8327" stopColor="#FF1E08" />
+            <stop offset="1" stopColor="#FF0000" />
+          </linearGradient>
+        </defs>
+      </svg>
+    </a>
+  );
+};
 
 Logo.propTypes = {
   dataId: T.string,
-  theme: T.object,
   href: T.string,
 };
 

diff --git a/src/core/Logo/component.rb b/src/core/Logo/component.rb
@@ -1,14 +1,17 @@
+require 'securerandom'
+
 module AblyUi
   module Core
     class Logo < ViewComponent::Base
       include AblyUi::Core::MeganavConfig
 
-      attr_reader :href, :data_id
+      attr_reader :href, :data_id, :gradient_id_0, :gradient_id_1
 
-      def initialize(theme_name:, data_id: '', href:)
+      def initialize(data_id: '', href:)
         @data_id = data_id
         @href = href
-        theme_setup(theme_name)
+        @gradient_id_0 = "paint_linear_#{SecureRandom.uuid}"
+        @gradient_id_1 = "paint_linear_#{SecureRandom.uuid}"
       end
 
       def logo_href

diff --git a/src/core/Meganav/component.html.erb b/src/core/Meganav/component.html.erb
@@ -2,7 +2,7 @@
   <%= notice %>
 
   <div class="ui-meganav ui-grid-px">
-    <%= render(AblyUi::Core::Logo.new(theme_name: @theme_name, data_id: "meganav-logo", href: logo_link)) %>
+    <%= render(AblyUi::Core::Logo.new(data_id: "meganav-logo", href: logo_link)) %>
     <%= render(AblyUi::Core::MeganavItemsDesktop.new(theme_name: @theme_name, url_base: url_base)) %>
 
     <% if @session_data[:signedIn] %>

diff --git a/src/core/Meganav/component.jsx b/src/core/Meganav/component.jsx
@@ -81,7 +81,7 @@ export default function Meganav({ paths, themeName = "white", notice, loginLink
     <nav className={`ui-meganav-wrapper ${theme.backgroundColor} ${theme.barShadow}`} data-id="meganav" aria-label="Main">
       {notice && <Notice {...notice.props} config={notice.config} />}
       <div className="ui-meganav ui-grid-px">
-        <Logo theme={theme} dataId="meganav-logo" href={urlBase} />
+        <Logo dataId="meganav-logo" href={urlBase} />
         <MeganavItemsDesktop panels={panels} paths={paths} theme={theme} absUrl={absUrl} />
 
         {/* Because we load the session state through fetch, we display a placeholder until fetch returns */}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -36,4 +36,4 @@ gem 'view_component', '~> 2.33.0', require: 'view_component/engine'

		gem 'responders'

		gem 'ably-ui', '7.8.0', require: 'ably_ui'
		gem 'ably-ui', '7.8.0.dev.502e5bf', require: 'ably_ui'