From 7845423bb95dbb299b6061a0e6f0d3b23d76c366 Mon Sep 17 00:00:00 2001 From: Fernando Valverde Date: Fri, 28 May 2021 15:10:15 -0600 Subject: [PATCH] Remove :runtime_banner feature flag (#13662) * Removes :runtime_banner feature flag + repositioning * Switch to https://udl.forem.com + fix spec * Fix URL specs * Adds data_update_script to remove feature flag * Fix banner overlay on reactions for small screens + iPad max width * Append -only to Runtime Filter CSS * Add import to fix assets:precompile * Implement design update * Apply suggestions from code review Co-authored-by: Lisa Sy * Add ; * Trigger Travis Co-authored-by: Lisa Sy --- app/assets/images/logo-forem-app.svg | 7 +- .../initializers/initializeRuntimeBanner.js | 2 +- app/assets/stylesheets/runtime-banner.scss | 45 ++++++++++- app/assets/stylesheets/runtime.scss | 78 +++++++++---------- app/lib/url.rb | 2 +- app/views/layouts/application.html.erb | 25 +++--- ...2235_remove_runtime_banner_feature_flag.rb | 7 ++ ...remove_runtime_banner_feature_flag_spec.rb | 20 +++++ spec/lib/url_spec.rb | 4 +- spec/requests/chat_channels_spec.rb | 8 +- spec/requests/registrations_spec.rb | 1 - spec/system/user/user_edits_profile_spec.rb | 1 - 12 files changed, 135 insertions(+), 65 deletions(-) create mode 100644 lib/data_update_scripts/20210506052235_remove_runtime_banner_feature_flag.rb create mode 100644 spec/lib/data_update_scripts/remove_runtime_banner_feature_flag_spec.rb diff --git a/app/assets/images/logo-forem-app.svg b/app/assets/images/logo-forem-app.svg index 2d00f3890..1d3491e4f 100644 --- a/app/assets/images/logo-forem-app.svg +++ b/app/assets/images/logo-forem-app.svg @@ -1,4 +1,5 @@ - - - + + + + diff --git a/app/assets/javascripts/initializers/initializeRuntimeBanner.js b/app/assets/javascripts/initializers/initializeRuntimeBanner.js index e62610de1..176980f56 100644 --- a/app/assets/javascripts/initializers/initializeRuntimeBanner.js +++ b/app/assets/javascripts/initializers/initializeRuntimeBanner.js @@ -56,7 +56,7 @@ function initializeRuntimeBanner() { // Constants - they will become dynamic (configurable by creators) in upcoming releases const FOREM_IOS_SCHEME = 'com.forem.app'; const FOREM_APP_STORE_URL = - 'https://apps.apple.com/us/app/dev-community/id1439094790'; + 'https://apps.apple.com/us/app/forem/id1536933197'; const FOREM_GOOGLE_PLAY_URL = 'https://play.google.com/store/apps/details?id=to.dev.dev_android'; diff --git a/app/assets/stylesheets/runtime-banner.scss b/app/assets/stylesheets/runtime-banner.scss index 0e25113cb..596dcc084 100644 --- a/app/assets/stylesheets/runtime-banner.scss +++ b/app/assets/stylesheets/runtime-banner.scss @@ -1,5 +1,6 @@ @import 'variables'; @import '_mixins'; +@import 'config/import'; .runtime-banner { display: flex; @@ -7,20 +8,56 @@ left: var(--su-4); right: var(--su-4); position: fixed; - bottom: calc(var(--su-9) + env(safe-area-inset-bottom)); + bottom: calc(var(--su-2) + env(safe-area-inset-bottom)); flex-flow: row nowrap; align-items: center; - padding: var(--su-1); - max-width: calc(100% - var(--su-4) * 2); + padding: var(--su-3); + max-width: calc(min(100% - var(--su-4) * 2, 750px)); z-index: var(--z-popover); border-radius: var(--radius); - background: var(--snackbar-bg); + background: #012026; color: var(--snackbar-color); font-size: var(--fs-s); :any-link { color: inherit; } + + a { + svg { + border: 1px solid #ffffff; + box-sizing: border-box; + border-radius: 3px; + } + + div > span:first-child { + font-size: 16px; + line-height: 16px; + color: #ffffff; + margin-bottom: var(--su-1); + } + + div > span:last-child { + font-size: 14px; + line-height: 14px; + color: #ffffff; + opacity: 0.65; + } + } + + // Close button + button > svg { + color: #ffffff; + } +} + +// Larger devices (i.e. iPad) display the reactions to the left instead of the +// bottom of the screen. Because of this we only need to move the banner higher +// up to avoid blocking the reaction buttons when size is <= $breakpoint-m +@media (max-width: $breakpoint-m) { + div[data-current-page='stories-show'] .runtime-banner { + bottom: calc(var(--su-9) + env(safe-area-inset-bottom)); + } } .mobile-deep-link-banner { diff --git a/app/assets/stylesheets/runtime.scss b/app/assets/stylesheets/runtime.scss index 821fa2909..8aebbe0ed 100644 --- a/app/assets/stylesheets/runtime.scss +++ b/app/assets/stylesheets/runtime.scss @@ -8,89 +8,89 @@ // More thorough context can be found in: https://github.com/forem/rfcs/pull/27 // Elements with all of these classes are not visible by default -.ForemWebView, -.ForemWebView-iOS, -.ForemWebView-Android, -.PWA, -.PWA-iOS, -.PWA-Android, -.PWA-Windows, -.PWA-Linux, -.PWA-macOS, -.Browser, -.Browser-iOS, -.Browser-Android, -.Browser-macOS, -.Browser-Windows, -.Browser-Linux { +.ForemWebView-only, +.ForemWebView-iOS-only, +.ForemWebView-Android-only, +.PWA-only, +.PWA-iOS-only, +.PWA-Android-only, +.PWA-Windows-only, +.PWA-Linux-only, +.PWA-macOS-only, +.Browser-only, +.Browser-iOS-only, +.Browser-Android-only, +.Browser-macOS-only, +.Browser-Windows-only, +.Browser-Linux-only { display: none; } // Elements with class 'ForemWebView' should be visible on any attribute that // matches that Medium regardless of OS -body[data-runtime='ForemWebView-iOS'] .ForemWebView, -body[data-runtime='ForemWebView-Android'] .ForemWebView { +body[data-runtime='ForemWebView-iOS'] .ForemWebView-only, +body[data-runtime='ForemWebView-Android'] .ForemWebView-only { display: var(--runtime-display, block); } // Elements that need to match both 'ForemWebView' Medium and OS -body[data-runtime='ForemWebView-iOS'] .ForemWebView-iOS { +body[data-runtime='ForemWebView-iOS'] .ForemWebView-iOS-only { display: var(--runtime-display, block); } -body[data-runtime='ForemWebView-Android'] .ForemWebView-Android { +body[data-runtime='ForemWebView-Android'] .ForemWebView-Android-only { display: var(--runtime-display, block); } // Elements with class 'Browser' should be visible on any attribute that // matches that Medium regardless of OS -body[data-runtime='Browser-Android'] .Browser, -body[data-runtime='Browser-iOS'] .Browser, -body[data-runtime='Browser-Linux'] .Browser, -body[data-runtime='Browser-Windows'] .Browser, -body[data-runtime='Browser-macOS'] .Browser { +body[data-runtime='Browser-Android'] .Browser-only, +body[data-runtime='Browser-iOS'] .Browser-only, +body[data-runtime='Browser-Linux'] .Browser-only, +body[data-runtime='Browser-Windows'] .Browser-only, +body[data-runtime='Browser-macOS'] .Browser-only { display: var(--runtime-display, block); } // Elements that need to match both 'Browser' Medium and OS -body[data-runtime='Browser-Android'] .Browser-Android { +body[data-runtime='Browser-Android'] .Browser-Android-only { display: var(--runtime-display, block); } -body[data-runtime='Browser-iOS'] .Browser-iOS { +body[data-runtime='Browser-iOS'] .Browser-iOS-only { display: var(--runtime-display, block); } -body[data-runtime='Browser-Linux'] .Browser-Linux { +body[data-runtime='Browser-Linux'] .Browser-Linux-only { display: var(--runtime-display, block); } -body[data-runtime='Browser-Windows'] .Browser-Windows { +body[data-runtime='Browser-Windows'] .Browser-Windows-only { display: var(--runtime-display, block); } -body[data-runtime='Browser-macOS'] .Browser-macOS { +body[data-runtime='Browser-macOS'] .Browser-macOS-only { display: var(--runtime-display, block); } // Elements with class 'PWA' should be visible on any attribute that // matches that Medium regardless of OS -body[data-runtime='PWA-Android'] .PWA, -body[data-runtime='PWA-iOS'] .PWA, -body[data-runtime='PWA-Linux'] .PWA, -body[data-runtime='PWA-Windows'] .PWA, -body[data-runtime='PWA-macOS'] .PWA { +body[data-runtime='PWA-Android'] .PWA-only, +body[data-runtime='PWA-iOS'] .PWA-only, +body[data-runtime='PWA-Linux'] .PWA-only, +body[data-runtime='PWA-Windows'] .PWA-only, +body[data-runtime='PWA-macOS'] .PWA-only { display: var(--runtime-display, block); } // Elements that need to match both 'PWA' Medium and OS -body[data-runtime='PWA-Android'] .PWA-Android { +body[data-runtime='PWA-Android'] .PWA-Android-only { display: var(--runtime-display, block); } -body[data-runtime='PWA-iOS'] .PWA-iOS { +body[data-runtime='PWA-iOS'] .PWA-iOS-only { display: var(--runtime-display, block); } -body[data-runtime='PWA-Linux'] .PWA-Linux { +body[data-runtime='PWA-Linux'] .PWA-Linux-only { display: var(--runtime-display, block); } -body[data-runtime='PWA-Windows'] .PWA-Windows { +body[data-runtime='PWA-Windows'] .PWA-Windows-only { display: var(--runtime-display, block); } -body[data-runtime='PWA-macOS'] .PWA-macOS { +body[data-runtime='PWA-macOS'] .PWA-macOS-only { display: var(--runtime-display, block); } diff --git a/app/lib/url.rb b/app/lib/url.rb index 6043dc43f..3948b493a 100644 --- a/app/lib/url.rb +++ b/app/lib/url.rb @@ -74,7 +74,7 @@ module URL # @param path [String] the target path to deep link def self.deep_link(path) target_path = CGI.escape(url("/r/mobile?deep_link=#{path}")) - "https://forem-udl-server.herokuapp.com/?r=#{target_path}" + "https://udl.forem.com/?r=#{target_path}" end def self.organization(organization) diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 06278813b..95fb8ad1f 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -104,19 +104,20 @@ <% end %> <%= yield %> - <% if FeatureFlag.enabled?(:runtime_banner) || ENV['E2E'] == 'true' %> -
-
- - <%= inline_svg_tag("logo-forem-app.svg", aria_hidden: true, class: "crayons-icon crayons-icon--default", width: 32, height: 32) %> - Open with the Forem app... - - -
+
+
+ + <%= inline_svg_tag("logo-forem-app.svg", aria_hidden: true, class: "crayons-icon crayons-icon--default", width: 32, height: 32) %> +
+ Forem + Open with the Forem app +
+ +
- <% end %> +
<% unless internal_navigation? %> diff --git a/lib/data_update_scripts/20210506052235_remove_runtime_banner_feature_flag.rb b/lib/data_update_scripts/20210506052235_remove_runtime_banner_feature_flag.rb new file mode 100644 index 000000000..7dd290470 --- /dev/null +++ b/lib/data_update_scripts/20210506052235_remove_runtime_banner_feature_flag.rb @@ -0,0 +1,7 @@ +module DataUpdateScripts + class RemoveRuntimeBannerFeatureFlag + def run + FeatureFlag.remove(:runtime_banner) + end + end +end diff --git a/spec/lib/data_update_scripts/remove_runtime_banner_feature_flag_spec.rb b/spec/lib/data_update_scripts/remove_runtime_banner_feature_flag_spec.rb new file mode 100644 index 000000000..cca1750c7 --- /dev/null +++ b/spec/lib/data_update_scripts/remove_runtime_banner_feature_flag_spec.rb @@ -0,0 +1,20 @@ +require "rails_helper" +require Rails.root.join( + "lib/data_update_scripts/20210506052235_remove_runtime_banner_feature_flag.rb", +) + +describe DataUpdateScripts::RemoveRuntimeBannerFeatureFlag do + it "removes the :runtime_banner flag" do + FeatureFlag.enable(:runtime_banner) + + described_class.new.run + + expect(FeatureFlag.exist?(:runtime_banner)).to be(false) + end + + it "works if the flag is not available" do + described_class.new.run + + expect(FeatureFlag.exist?(:runtime_banner)).to be(false) + end +end diff --git a/spec/lib/url_spec.rb b/spec/lib/url_spec.rb index fd7f6ed5e..20293b954 100644 --- a/spec/lib/url_spec.rb +++ b/spec/lib/url_spec.rb @@ -105,11 +105,11 @@ RSpec.describe URL, type: :lib do describe ".deep_link" do it "returns the correct URL for the root path" do - expect(described_class.deep_link("/")).to eq("https://forem-udl-server.herokuapp.com/?r=https%3A%2F%2Fdev.to%2Fr%2Fmobile%3Fdeep_link%3D%2F") + expect(described_class.deep_link("/")).to eq("https://udl.forem.com/?r=https%3A%2F%2Fdev.to%2Fr%2Fmobile%3Fdeep_link%3D%2F") end it "returns the correct URL for an explicit path" do - expect(described_class.deep_link("/sloan")).to eq("https://forem-udl-server.herokuapp.com/?r=https%3A%2F%2Fdev.to%2Fr%2Fmobile%3Fdeep_link%3D%2Fsloan") + expect(described_class.deep_link("/sloan")).to eq("https://udl.forem.com/?r=https%3A%2F%2Fdev.to%2Fr%2Fmobile%3Fdeep_link%3D%2Fsloan") end end diff --git a/spec/requests/chat_channels_spec.rb b/spec/requests/chat_channels_spec.rb index 49bcd5c0c..20cb4c1e8 100644 --- a/spec/requests/chat_channels_spec.rb +++ b/spec/requests/chat_channels_spec.rb @@ -51,7 +51,13 @@ RSpec.describe "ChatChannels", type: :request do it "have no active channel" do expect(response).not_to(redirect_to(connect_path(invite_channel.slug))) - expect(response.body).not_to include(invite_channel.slug) + # The slug will be rendered by the mobile runtime banner because the + # request is made directly to "/connect/#{invite_channel.slug}", that's + # why we can't simply check that the slug isn't included in + # `response.body`. We can check using a regex to make sure the channel + # doesn't exist within the chat (after chat DOM element appears but also + # before the runtime banner starts). + expect(response.body).not_to match(/