diff --git a/app/assets/javascripts/admin/controllers/admin-site-settings.js.es6 b/app/assets/javascripts/admin/controllers/admin-site-settings.js.es6 index 25420b562dd..9473cbccae5 100644 --- a/app/assets/javascripts/admin/controllers/admin-site-settings.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-site-settings.js.es6 @@ -17,7 +17,9 @@ export default Ember.Controller.extend({ if ((!filter || 0 === filter.length) && !this.get("onlyOverridden")) { this.set("visibleSiteSettings", this.get("allSiteSettings")); - this.transitionToRoute("adminSiteSettings"); + if (this.get("categoryNameKey") === "all_results") { + this.transitionToRoute("adminSiteSettings"); + } return; } @@ -77,7 +79,7 @@ export default Ember.Controller.extend({ } else { this.filterContentNow(); } - }, 250).observes("filter", "onlyOverridden"), + }, 250).observes("filter", "onlyOverridden", "model"), actions: { clearFilter() { diff --git a/app/assets/javascripts/admin/mixins/setting-component.js.es6 b/app/assets/javascripts/admin/mixins/setting-component.js.es6 index c3f4f19e3d3..9f8166fbecf 100644 --- a/app/assets/javascripts/admin/mixins/setting-component.js.es6 +++ b/app/assets/javascripts/admin/mixins/setting-component.js.es6 @@ -113,6 +113,7 @@ export default Ember.Mixin.create({ .then(() => { this.set("validationMessage", null); this.commitBuffer(); + this.afterSave(); }) .catch(e => { if (e.jqXHR.responseJSON && e.jqXHR.responseJSON.errors) { diff --git a/app/assets/javascripts/admin/routes/admin-site-settings-category.js.es6 b/app/assets/javascripts/admin/routes/admin-site-settings-category.js.es6 index c90a099318f..a7744150830 100644 --- a/app/assets/javascripts/admin/routes/admin-site-settings-category.js.es6 +++ b/app/assets/javascripts/admin/routes/admin-site-settings-category.js.es6 @@ -5,6 +5,10 @@ export default Discourse.Route.extend({ "categoryNameKey", params.category_id ); + this.controllerFor("adminSiteSettings").set( + "categoryNameKey", + params.category_id + ); return Ember.Object.create({ nameKey: params.category_id, name: I18n.t("admin.site_settings.categories." + params.category_id), diff --git a/app/assets/javascripts/admin/routes/admin-site-settings.js.es6 b/app/assets/javascripts/admin/routes/admin-site-settings.js.es6 index 58ac7246f53..b6c3e857ea9 100644 --- a/app/assets/javascripts/admin/routes/admin-site-settings.js.es6 +++ b/app/assets/javascripts/admin/routes/admin-site-settings.js.es6 @@ -15,5 +15,13 @@ export default Discourse.Route.extend({ if (!controller.get("visibleSiteSettings")) { controller.set("visibleSiteSettings", siteSettings); } + }, + + actions: { + refreshAll() { + SiteSetting.findAll().then(settings => { + this.controllerFor("adminSiteSettings").set("model", settings); + }); + } } }); diff --git a/app/assets/javascripts/admin/templates/components/site-settings/upload.hbs b/app/assets/javascripts/admin/templates/components/site-settings/upload.hbs index d13892157db..51711eee272 100644 --- a/app/assets/javascripts/admin/templates/components/site-settings/upload.hbs +++ b/app/assets/javascripts/admin/templates/components/site-settings/upload.hbs @@ -1,2 +1,2 @@ -{{site-settings-image-uploader imageUrl=value type="site_setting"}} +{{site-settings-image-uploader imageUrl=value placeholderUrl=setting.placeholder type="site_setting"}}
{{{unbound setting.description}}}
diff --git a/app/assets/javascripts/admin/templates/site-settings-category.hbs b/app/assets/javascripts/admin/templates/site-settings-category.hbs index 5f612419326..928daa224a9 100644 --- a/app/assets/javascripts/admin/templates/site-settings-category.hbs +++ b/app/assets/javascripts/admin/templates/site-settings-category.hbs @@ -1,7 +1,7 @@ {{#if filteredContent}} {{#d-section class="form-horizontal settings"}} {{#each filteredContent as |setting|}} - {{site-setting setting=setting}} + {{site-setting setting=setting afterSave=(route-action "refreshAll")}} {{/each}} {{#if category.hasMore}}

{{i18n 'admin.site_settings.more_than_30_results'}}

diff --git a/app/assets/javascripts/discourse/components/image-uploader.js.es6 b/app/assets/javascripts/discourse/components/image-uploader.js.es6 index 5d468b15e6e..234de6e8434 100644 --- a/app/assets/javascripts/discourse/components/image-uploader.js.es6 +++ b/app/assets/javascripts/discourse/components/image-uploader.js.es6 @@ -21,13 +21,25 @@ export default Ember.Component.extend(UploadMixin, { } }, - @computed("imageUrl") - backgroundStyle(imageUrl) { - if (Ember.isEmpty(imageUrl)) { + @computed("imageUrl", "placeholderUrl") + showingPlaceholder(imageUrl, placeholderUrl) { + return !imageUrl && placeholderUrl; + }, + + @computed("placeholderUrl") + placeholderStyle(url) { + if (Ember.isEmpty(url)) { return "".htmlSafe(); } + return `background-image: url(${url})`.htmlSafe(); + }, - return `background-image: url(${imageUrl})`.htmlSafe(); + @computed("imageUrl") + backgroundStyle(url) { + if (Ember.isEmpty(url)) { + return "".htmlSafe(); + } + return `background-image: url(${url})`.htmlSafe(); }, @computed("imageUrl") @@ -36,11 +48,6 @@ export default Ember.Component.extend(UploadMixin, { return imageUrl.split("/").slice(-1)[0]; }, - @computed("backgroundStyle") - hasBackgroundStyle(backgroundStyle) { - return !Ember.isEmpty(backgroundStyle.string); - }, - validateUploadedFilesOptions() { return { imagesOnly: true }; }, diff --git a/app/assets/javascripts/discourse/templates/components/image-uploader.hbs b/app/assets/javascripts/discourse/templates/components/image-uploader.hbs index c795a851a5a..716f4c646a6 100644 --- a/app/assets/javascripts/discourse/templates/components/image-uploader.hbs +++ b/app/assets/javascripts/discourse/templates/components/image-uploader.hbs @@ -1,11 +1,14 @@
+ {{#if showingPlaceholder}} +
+ {{/if}}
- {{#if hasBackgroundStyle}} + {{#if imageUrl}} {{/if}} diff --git a/app/assets/javascripts/wizard/components/image-preview-apple-touch-icon.js.es6 b/app/assets/javascripts/wizard/components/image-preview-large-icon.js.es6 similarity index 100% rename from app/assets/javascripts/wizard/components/image-preview-apple-touch-icon.js.es6 rename to app/assets/javascripts/wizard/components/image-preview-large-icon.js.es6 diff --git a/app/assets/javascripts/wizard/controllers/step.js.es6 b/app/assets/javascripts/wizard/controllers/step.js.es6 index 06110b894d5..7c632570392 100644 --- a/app/assets/javascripts/wizard/controllers/step.js.es6 +++ b/app/assets/javascripts/wizard/controllers/step.js.es6 @@ -1,5 +1,3 @@ -import getUrl from "discourse-common/lib/get-url"; - export default Ember.Controller.extend({ wizard: null, step: null, @@ -8,10 +6,9 @@ export default Ember.Controller.extend({ goNext(response) { const next = this.get("step.next"); if (response.refresh_required) { - document.location = getUrl(`/wizard/steps/${next}`); - } else { - this.transitionToRoute("step", next); + this.send("refresh"); } + this.transitionToRoute("step", next); }, goBack() { this.transitionToRoute("step", this.get("step.previous")); diff --git a/app/assets/javascripts/wizard/routes/application.js.es6 b/app/assets/javascripts/wizard/routes/application.js.es6 index ed366714b29..d460cdcc9af 100644 --- a/app/assets/javascripts/wizard/routes/application.js.es6 +++ b/app/assets/javascripts/wizard/routes/application.js.es6 @@ -3,5 +3,11 @@ import { findWizard } from "wizard/models/wizard"; export default Ember.Route.extend({ model() { return findWizard(); + }, + + actions: { + refresh() { + this.refresh(); + } } }); diff --git a/app/assets/stylesheets/common/base/upload.scss b/app/assets/stylesheets/common/base/upload.scss index d52ec8cf41a..8eebba747a3 100644 --- a/app/assets/stylesheets/common/base/upload.scss +++ b/app/assets/stylesheets/common/base/upload.scss @@ -3,7 +3,20 @@ background-size: cover; position: relative; + .placeholder-overlay { + background-size: contain; + background-repeat: no-repeat; + background-position: center; + position: absolute; + top: 0; + left: 0; + width: 100%; + height: 100%; + opacity: 0.3; + } + .image-upload-controls { + position: relative; display: flex; .btn { diff --git a/app/controllers/metadata_controller.rb b/app/controllers/metadata_controller.rb index e7d16a58af9..0e7f02a2f63 100644 --- a/app/controllers/metadata_controller.rb +++ b/app/controllers/metadata_controller.rb @@ -13,16 +13,6 @@ class MetadataController < ApplicationController private def default_manifest - logo = SiteSetting.site_large_icon_url.presence || - SiteSetting.site_logo_small_url.presence || - SiteSetting.site_apple_touch_icon_url.presence - - if !logo - logo = '/images/d-logo-sketch-small.png' - end - - file_info = get_file_info(logo) - display = Regexp.new(SiteSetting.pwa_display_browser_regex).match(request.user_agent) ? 'browser' : 'standalone' manifest = { @@ -32,11 +22,6 @@ class MetadataController < ApplicationController background_color: "##{ColorScheme.hex_for_name('secondary', view_context.scheme_id)}", theme_color: "##{ColorScheme.hex_for_name('header_background', view_context.scheme_id)}", icons: [ - { - src: UrlHelper.absolute(logo), - sizes: file_info[:size], - type: file_info[:type] - } ], share_target: { action: "/new-topic", @@ -49,6 +34,13 @@ class MetadataController < ApplicationController } } + logo = SiteSetting.site_manifest_icon_url + manifest[:icons] << { + src: UrlHelper.absolute(logo), + sizes: "512x512", + type: MiniMime.lookup_by_filename(logo)&.content_type || "image/png" + } if logo + manifest[:short_name] = SiteSetting.short_title if SiteSetting.short_title.present? if current_user && current_user.trust_level >= 1 && SiteSetting.native_app_install_banner_android @@ -66,10 +58,4 @@ class MetadataController < ApplicationController manifest end - def get_file_info(filename) - type = MiniMime.lookup_by_filename(filename)&.content_type || "image/png" - upload = Upload.find_by_url(filename) - { size: "#{upload&.width || 512}x#{upload&.height || 512}", type: type } - end - end diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb index 95b4be02204..f86b02beb0f 100644 --- a/app/controllers/static_controller.rb +++ b/app/controllers/static_controller.rb @@ -124,8 +124,8 @@ class StaticController < ApplicationController is_asset_path hijack do - data = DistributedMemoizer.memoize("FAVICON#{SiteSetting.site_favicon_url}", 60 * 30) do - favicon = SiteSetting.favicon + data = DistributedMemoizer.memoize("FAVICON#{SiteIconManager.favicon_url}", 60 * 30) do + favicon = SiteIconManager.favicon next "" unless favicon if Discourse.store.external? diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index ac7b20cfe2c..df7237c9aad 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -220,11 +220,7 @@ module ApplicationHelper opts[:twitter_summary_large_image] = twitter_summary_large_image_url end - opts[:image] = SiteSetting.site_opengraph_image_url.presence || - twitter_summary_large_image_url.presence || - SiteSetting.site_large_icon_url.presence || - SiteSetting.site_apple_touch_icon_url.presence || - SiteSetting.site_logo_url.presence + opts[:image] = SiteSetting.site_opengraph_image_url end # Use the correct scheme for opengraph/twitter image diff --git a/app/models/admin_dashboard_data.rb b/app/models/admin_dashboard_data.rb index f9639e913ee..294486e1622 100644 --- a/app/models/admin_dashboard_data.rb +++ b/app/models/admin_dashboard_data.rb @@ -90,7 +90,7 @@ class AdminDashboardData add_problem_check :rails_env_check, :host_names_check, :force_https_check, :ram_check, :google_oauth2_config_check, :facebook_config_check, :twitter_config_check, - :github_config_check, :pwa_config_check, :s3_config_check, + :github_config_check, :s3_config_check, :image_magick_check, :failing_emails_check, :subfolder_ends_in_slash_check, :pop3_polling_configuration, :email_polling_errored_recently, @@ -172,15 +172,6 @@ class AdminDashboardData end end - def pwa_config_check - unless SiteSetting.large_icon.present? && SiteSetting.large_icon.width == 512 && SiteSetting.large_icon.height == 512 - return I18n.t('dashboard.pwa_config_icon_warning', base_path: Discourse.base_path) - end - unless SiteSetting.short_title.present? && SiteSetting.short_title.size <= 12 - return I18n.t('dashboard.pwa_config_title_warning', base_path: Discourse.base_path) - end - end - def s3_config_check # if set via global setting it is validated during the `use_s3?` call if !GlobalSetting.use_s3? diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 124259fdc79..88201bbdcad 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -187,6 +187,7 @@ class SiteSetting < ActiveRecord::Base digest_logo mobile_logo large_icon + manifest_icon favicon apple_touch_icon twitter_summary_large_image @@ -194,6 +195,10 @@ class SiteSetting < ActiveRecord::Base push_notifications_icon }.each do |setting_name| define_singleton_method("site_#{setting_name}_url") do + if SiteIconManager.respond_to?("#{setting_name}_url") + return SiteIconManager.public_send("#{setting_name}_url") + end + upload = self.public_send(setting_name) upload ? full_cdn_url(upload.url) : '' end diff --git a/config/initializers/014-track-setting-changes.rb b/config/initializers/014-track-setting-changes.rb index 41a7374571c..49259ed2e6a 100644 --- a/config/initializers/014-track-setting-changes.rb +++ b/config/initializers/014-track-setting-changes.rb @@ -1,3 +1,5 @@ +require_dependency "site_icon_manager" + DiscourseEvent.on(:site_setting_changed) do |name, old_value, new_value| # Enabling `must_approve_users` on an existing site is odd, so we assume that the # existing users are approved. @@ -31,4 +33,8 @@ DiscourseEvent.on(:site_setting_changed) do |name, old_value, new_value| Jobs.enqueue(:update_s3_inventory) if [:s3_inventory, :s3_upload_bucket].include?(name) SvgSprite.expire_cache if name.to_s.include?("_icon") + + if SiteIconManager::WATCHED_SETTINGS.include?(name) + SiteIconManager.ensure_optimized! + end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 329a8585711..d28fd571658 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4121,6 +4121,7 @@ en: categories: all_results: "All" required: "Required" + branding: "Branding" basic: "Basic Setup" users: "Users" posting: "Posting" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c3332cf26fb..52e2302867e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1270,8 +1270,6 @@ en: facebook_config_warning: 'The server is configured to allow signup and log in with Facebook (enable_facebook_logins), but the app id and app secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' twitter_config_warning: 'The server is configured to allow signup and log in with Twitter (enable_twitter_logins), but the key and secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' github_config_warning: 'The server is configured to allow signup and log in with GitHub (enable_github_logins), but the client id and secret values are not set. Go to the Site Settings and update the settings. See this guide to learn more.' - pwa_config_icon_warning: 'Your site is missing a 512 × 512 icon which allows users to add a homescreen shortcut to this site on Android devices. Go to the Site Settings and upload a 512 × 512 icon.' - pwa_config_title_warning: 'Your site is missing a short title which allows users to add a homescreen shortcut to your site on Android devices. Go to the Site Settings and add a short title, limited to 12 characters.' s3_config_warning: 'The server is configured to upload files to S3, but at least one the following setting is not set: s3_access_key_id, s3_secret_access_key, s3_use_iam_profile, or s3_upload_bucket. Go to the Site Settings and update the settings. See "How to set up image uploads to S3?" to learn more.' s3_backup_config_warning: 'The server is configured to upload backups to S3, but at least one the following setting is not set: s3_access_key_id, s3_secret_access_key, s3_use_iam_profile, or s3_backup_bucket. Go to the Site Settings and update the settings. See "How to set up image uploads to S3?" to learn more.' image_magick_warning: 'The server is configured to create thumbnails of large images, but ImageMagick is not installed. Install ImageMagick using your favorite package manager or download the latest release.' @@ -1359,11 +1357,12 @@ en: logo_small: "The small logo image at the top left of your site, seen when scrolling down. Use a square 120 × 120 image. If left blank, a home glyph will be shown." digest_logo: "The alternate logo image used at the top of your site's email summary. Use a wide rectangle image. Don't use an SVG image. If left blank, the image from the `logo` setting will be used." mobile_logo: "The logo used on mobile version of your site. Use a wide rectangular image with a height of 120 and an aspect ratio greater than 3:1. If left blank, the image from the `logo` setting will be used." - large_icon: "Image used as logo/splash image on Android. Required size is 512 × 512." - favicon: "A favicon for your site, see https://en.wikipedia.org/wiki/Favicon. To work correctly over a CDN it must be a png." - apple_touch_icon: "Icon used for Apple touch devices. Required size is 144 × 144." - opengraph_image: "Default opengraph image, used when the page has no other suitable image or site logo." - twitter_summary_large_image: "Default Twitter summary card image (should be at least 280 in width, and at least 150 in height)." + large_icon: "Image used as the base for other metadata icons. Should ideally be larger than 512 x 512. If left blank, logo_small will be used." + manifest_icon: "Image used as logo/splash image on Android. Will be automatically resized to 512 × 512. If left blank, large_icon will be used." + favicon: "A favicon for your site, see https://en.wikipedia.org/wiki/Favicon. To work correctly over a CDN it must be a png. Will be resized to 32x32. If left blank, large_icon will be used." + apple_touch_icon: "Icon used for Apple touch devices. Will be automatically resized to 180x180. If left blank, large_icon will be used." + opengraph_image: "Default opengraph image, used when the page has no other suitable image. If left blank, large_icon will be used" + twitter_summary_large_image: "Twitter card 'summary large image' (should be at least 280 in width, and at least 150 in height). If left blank, regular card metadata is generated using the opengraph_image." notification_email: "The from: email address used when sending all essential system emails. The domain specified here must have SPF, DKIM and reverse PTR records set correctly for email to arrive." email_custom_headers: "A pipe-delimited list of custom email headers" @@ -4340,18 +4339,18 @@ en: label: "Primary Logo" description: "The logo image at the top left of your site. Use a wide rectangular image with a height of 120 and an aspect ratio greater than 3:1" logo_small: - label: "Compact Logo" - description: "A compact version of your logo, shown at the top left of your site when scrolling down. Use a square 120 × 120 image." + label: "Square Logo" + description: "A square version of your logo. Shown at the top left of your site when scrolling down, in the browser, and when sharing on social platforms. Ideally larger than 512x512." icons: title: "Icons" fields: favicon: - label: "Small Icon" - description: "Icon image used to represent your site in web browsers that looks good at small sizes such as 32 × 32. Recommended image extensions are PNG or JPG." - apple_touch_icon: + label: "Browser Icon" + description: "Icon image used to represent your site in web browsers that looks good at small sizes. Recommended image extensions are PNG or JPG. We'll use the square logo by default." + large_icon: label: "Large Icon" - description: "Icon image used to represent your site on modern devices that looks good at larger sizes. Required size is at least 512 × 512." + description: "Icon image used to represent your site on modern devices that looks good at larger sizes. Ideally larger than 512 × 512. We'll use the square logo by default." homepage: description: "We recommend showing the latest topics on your homepage, but you can also show categories (groups of topics) on the homepage if you prefer." diff --git a/config/site_settings.yml b/config/site_settings.yml index 564d909e9d3..ced98fe3244 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -50,15 +50,26 @@ required: site_contact_group_name: default: "" type: group + exclude_rel_nofollow_domains: + default: "" + type: list + company_name: + default: "" + governing_law: + default: "" + city_for_disputes: + default: "" + +branding: logo: - default: -1 + default: -5 client: true type: upload logo_url: hidden: true default: "/images/d-logo-sketch.png" logo_small: - default: -2 + default: -6 client: true type: upload logo_small_url: @@ -82,18 +93,21 @@ required: default: "" client: true type: upload + manifest_icon: + default: "" + type: upload large_icon_url: hidden: true default: "" favicon: - default: -3 + default: "" client: true type: upload favicon_url: hidden: true default: "/images/default-favicon.ico" apple_touch_icon: - default: -4 + default: "" client: true type: upload apple_touch_icon_url: @@ -111,15 +125,6 @@ required: twitter_summary_large_image_url: hidden: true default: "" - exclude_rel_nofollow_domains: - default: "" - type: list - company_name: - default: "" - governing_law: - default: "" - city_for_disputes: - default: "" basic: allow_user_locale: diff --git a/db/fixtures/010_uploads.rb b/db/fixtures/010_uploads.rb index 630ac04a5c0..2b3097f8b23 100644 --- a/db/fixtures/010_uploads.rb +++ b/db/fixtures/010_uploads.rb @@ -1,8 +1,10 @@ { - -1 => "d-logo-sketch.png", - -2 => "d-logo-sketch-small.png", - -3 => "default-favicon.ico", - -4 => "default-apple-touch-icon.png" + -1 => "d-logo-sketch.png", # Old version + -2 => "d-logo-sketch-small.png", # Old version + -3 => "default-favicon.ico", # No longer used + -4 => "default-apple-touch-icon.png", # No longer used + -5 => "discourse-logo-sketch.png", + -6 => "discourse-logo-sketch-small.png", }.each do |id, filename| path = Rails.root.join("public/images/#{filename}") @@ -13,5 +15,9 @@ upload.url = "/images/#{filename}" upload.filesize = File.size(path) upload.extension = File.extname(path)[1..10] + # Fake an SHA1. We need to have something, so that other parts of the application + # keep working. But we can't use the real SHA1, in case the seeded file has already + # been uploaded. Use an underscore to make clash impossible. + upload.sha1 = "_#{Upload.generate_digest(path)}"[0..Upload::SHA1_LENGTH - 1] end end diff --git a/lib/remap.rb b/lib/remap.rb index 47731c1514a..020cc2c2bba 100644 --- a/lib/remap.rb +++ b/lib/remap.rb @@ -37,6 +37,7 @@ WHERE table_schema='public' and (data_type like 'char%' or data_type like 'text% end Theme.expire_site_cache! + SiteIconManager.ensure_optimized! end def log(message) diff --git a/lib/site_icon_manager.rb b/lib/site_icon_manager.rb new file mode 100644 index 00000000000..51d6a9d6b7b --- /dev/null +++ b/lib/site_icon_manager.rb @@ -0,0 +1,75 @@ +module SiteIconManager + extend GlobalPath + + @cache = DistributedCache.new('icon_manager') + + SKETCH_LOGO_ID = -6 + + ICONS = { + digest_logo: { width: nil, height: nil, settings: [:digest_logo, :logo], fallback_to_sketch: false, resize_required: false }, + mobile_logo: { width: nil, height: nil, settings: [:mobile_logo, :logo], fallback_to_sketch: false, resize_required: false }, + large_icon: { width: nil, height: nil, settings: [:large_icon, :logo_small], fallback_to_sketch: true, resize_required: false }, + manifest_icon: { width: 512, height: 512, settings: [:manifest_icon, :large_icon, :logo_small], fallback_to_sketch: true, resize_required: true }, + favicon: { width: 32, height: 32, settings: [:favicon, :large_icon, :logo_small], fallback_to_sketch: true, resize_required: false }, + apple_touch_icon: { width: 180, height: 180, settings: [:apple_touch_icon, :large_icon, :logo_small], fallback_to_sketch: true, resize_required: false }, + opengraph_image: { width: nil, height: nil, settings: [:opengraph_image, :large_icon, :logo_small, :logo], fallback_to_sketch: true, resize_required: false }, + } + + WATCHED_SETTINGS = ICONS.keys + [:logo, :logo_small] + + def self.ensure_optimized! + unless @disabled + ICONS.each do |name, info| + icon = resolve_original(info) + + if info[:height] && info[:width] + OptimizedImage.create_for(icon, info[:width], info[:height]) + end + end + end + @cache.clear + end + + ICONS.each do |name, info| + define_singleton_method(name) do + icon = resolve_original(info) + if info[:height] && info[:width] + result = OptimizedImage.find_by(upload: icon, height: info[:height], width: info[:width]) + end + result = icon if !result && !info[:resize_required] + result + end + + define_singleton_method("#{name}_url") do + get_set_cache("#{name}_url") do + icon = self.public_send(name) + icon ? full_cdn_url(icon.url) : '' + end + end + end + + # Used in test mode + def self.disable + @disabled = true + end + + def self.enable + @disabled = false + end + + private + + def self.get_set_cache(key) + @cache[key] ||= yield + end + + def self.resolve_original(info) + info[:settings].each do |setting_name| + value = SiteSetting.send(setting_name) + return value if value + end + return Upload.find(SKETCH_LOGO_ID) if info[:fallback_to_sketch] + nil + end + +end diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index bcca40f6e6b..a46cc333f80 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -261,6 +261,8 @@ module SiteSettingExtension def placeholder(setting) if !I18n.t("site_settings.placeholder.#{setting}", default: "").empty? I18n.t("site_settings.placeholder.#{setting}") + elsif SiteIconManager.respond_to?("#{setting}_url") + SiteIconManager.public_send("#{setting}_url") end end diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index 2b0f1af5e81..0b464d8435e 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -36,6 +36,10 @@ task 'db:migrate' => ['environment', 'set_locale'] do |_, args| SeedFu.seed(DiscoursePluginRegistry.seed_paths) unless Discourse.skip_post_deployment_migrations? + puts + print "Optimizing site icons... " + SiteIconManager.ensure_optimized! + puts "Done" puts print "Recompiling theme fields... " ThemeField.force_recompilation! diff --git a/lib/wizard/builder.rb b/lib/wizard/builder.rb index 3e896ec9452..6fae1d47f23 100644 --- a/lib/wizard/builder.rb +++ b/lib/wizard/builder.rb @@ -177,42 +177,21 @@ class Wizard step.add_field(id: 'logo_small', type: 'image', value: SiteSetting.site_logo_small_url) step.on_update do |updater| - updater.apply_settings(:logo, :logo_small) + if SiteSetting.site_logo_url != updater.fields[:logo] || + SiteSetting.site_logo_small_url != updater.fields[:logo_small] + updater.apply_settings(:logo, :logo_small) + updater.refresh_required = true + end end end @wizard.append_step('icons') do |step| step.add_field(id: 'favicon', type: 'image', value: SiteSetting.site_favicon_url) - step.add_field(id: 'apple_touch_icon', type: 'image', value: SiteSetting.site_apple_touch_icon_url) + step.add_field(id: 'large_icon', type: 'image', value: SiteSetting.site_large_icon_url) step.on_update do |updater| - updater.apply_settings(:favicon) - - if updater.fields[:apple_touch_icon] != SiteSetting.apple_touch_icon - upload = Upload.find_by_url(updater.fields[:apple_touch_icon]) - dimensions = 180 # for apple touch icon - - if upload && upload.width > dimensions && upload.height > dimensions - updater.update_setting(:large_icon, upload) - - apple_touch_icon_optimized = OptimizedImage.create_for( - upload, - dimensions, - dimensions - ) - - original_file = File.new(Discourse.store.path_for(apple_touch_icon_optimized)) rescue nil - - if original_file - apple_touch_icon_upload = UploadCreator.new(original_file, upload.original_filename).create_for(@wizard.user.id) - updater.update_setting(:apple_touch_icon, apple_touch_icon_upload) - end - - apple_touch_icon_optimized.destroy! if apple_touch_icon_optimized.present? - else - updater.apply_settings(:apple_touch_icon) - end - end + updater.apply_settings(:favicon) if SiteSetting.site_favicon_url != updater.fields[:favicon] + updater.apply_settings(:large_icon) if SiteSetting.site_large_icon_url != updater.fields[:large_icon] end end diff --git a/public/images/discourse-logo-sketch-small.png b/public/images/discourse-logo-sketch-small.png new file mode 100644 index 00000000000..a87a18889c4 Binary files /dev/null and b/public/images/discourse-logo-sketch-small.png differ diff --git a/public/images/discourse-logo-sketch.png b/public/images/discourse-logo-sketch.png new file mode 100644 index 00000000000..6cb1f546cc8 Binary files /dev/null and b/public/images/discourse-logo-sketch.png differ diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 850295d5d88..d1f97bbb058 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -178,7 +178,7 @@ describe PostCreator do user_action = messages.find { |m| m.channel == "/u/#{p.user.username}" } expect(user_action).not_to eq(nil) - expect(messages.length).to eq(5) + expect(messages.filter { |m| m.channel != "/distributed_hash" }.length).to eq(5) end it 'extracts links from the post' do diff --git a/spec/components/site_icon_manager_spec.rb b/spec/components/site_icon_manager_spec.rb new file mode 100644 index 00000000000..37d84b99202 --- /dev/null +++ b/spec/components/site_icon_manager_spec.rb @@ -0,0 +1,47 @@ +require 'rails_helper' + +class GlobalPathInstance + extend GlobalPath +end + +describe SiteIconManager do + before do + SiteIconManager.enable + end + + let(:upload) do + UploadCreator.new(file_from_fixtures("smallest.png"), 'logo.png').create_for(Discourse.system_user.id) + end + + it "works correctly" do + SiteSetting.logo = nil + SiteSetting.logo_small = nil + + # Falls back to sketch for some icons + expect(SiteIconManager.favicon.upload_id).to eq(SiteIconManager::SKETCH_LOGO_ID) + expect(SiteIconManager.mobile_logo).to eq(nil) + + SiteSetting.logo_small = upload + + # Always resizes to 512x512 + manifest = SiteIconManager.manifest_icon + expect(manifest.upload_id).to eq(upload.id) + expect(manifest.width).to eq(512) + expect(manifest.height).to eq(512) + + # Always resizes to 32x32 + favicon = SiteIconManager.favicon + expect(favicon.upload_id).to eq(upload.id) + expect(favicon.width).to eq(32) + expect(favicon.height).to eq(32) + + # Don't resize + opengraph = SiteIconManager.opengraph_image + expect(opengraph).to eq(upload) + + # Site Setting integration + expect(SiteSetting.manifest_icon).to eq(nil) + expect(SiteSetting.site_manifest_icon_url).to eq(GlobalPathInstance.full_cdn_url(manifest.url)) + end + +end diff --git a/spec/components/wizard/step_updater_spec.rb b/spec/components/wizard/step_updater_spec.rb index 12ab0dcd8be..ccaab1d0029 100644 --- a/spec/components/wizard/step_updater_spec.rb +++ b/spec/components/wizard/step_updater_spec.rb @@ -262,7 +262,7 @@ describe Wizard::StepUpdater do updater = wizard.create_updater('icons', favicon: upload.url, - apple_touch_icon: upload2.url + large_icon: upload2.url ) updater.update @@ -270,16 +270,7 @@ describe Wizard::StepUpdater do expect(updater).to be_success expect(wizard.completed_steps?('icons')).to eq(true) expect(SiteSetting.favicon).to eq(upload) - expect(SiteSetting.apple_touch_icon).to eq(upload2) - end - - it "updates large_icon if the uploaded icon size is greater than 180x180" do - upload = Fabricate(:upload, width: 512, height: 512) - updater = wizard.create_updater('icons', apple_touch_icon: upload.url) - updater.update - - expect(updater).to be_success - expect(SiteSetting.large_icon).to eq(upload) + expect(SiteSetting.large_icon).to eq(upload2) end end diff --git a/spec/components/wizard/wizard_builder_spec.rb b/spec/components/wizard/wizard_builder_spec.rb index 9281247abbf..7d0ed2f239b 100644 --- a/spec/components/wizard/wizard_builder_spec.rb +++ b/spec/components/wizard/wizard_builder_spec.rb @@ -70,16 +70,16 @@ describe Wizard::Builder do upload2 = Fabricate(:upload) SiteSetting.favicon = upload - SiteSetting.apple_touch_icon = upload2 + SiteSetting.large_icon = upload2 fields = icons_step.fields favicon_field = fields.first - apple_touch_icon_field = fields.last + large_icon_field = fields.last expect(favicon_field.id).to eq('favicon') expect(favicon_field.value).to eq(GlobalPathInstance.full_cdn_url(upload.url)) - expect(apple_touch_icon_field.id).to eq('apple_touch_icon') - expect(apple_touch_icon_field.value).to eq(GlobalPathInstance.full_cdn_url(upload2.url)) + expect(large_icon_field.id).to eq('large_icon') + expect(large_icon_field.value).to eq(GlobalPathInstance.full_cdn_url(upload2.url)) end end diff --git a/spec/fixtures/images/logo.jpg b/spec/fixtures/images/logo.jpg new file mode 100644 index 00000000000..3ca525dfc67 Binary files /dev/null and b/spec/fixtures/images/logo.jpg differ diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index e0ce7bb22d3..fb54c60192a 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -290,20 +290,14 @@ describe ApplicationHelper do ) SiteSetting.large_icon = nil - - expect(helper.crawlable_meta_data).to include( - SiteSetting.site_apple_touch_icon_url - ) - - SiteSetting.apple_touch_icon = nil - SiteSetting.apple_touch_icon_url = nil + SiteSetting.logo_small = nil expect(helper.crawlable_meta_data).to include(SiteSetting.site_logo_url) SiteSetting.logo = nil SiteSetting.logo_url = nil - expect(helper.crawlable_meta_data).to_not include("/images") + expect(helper.crawlable_meta_data).to include(Upload.find(SiteIconManager::SKETCH_LOGO_ID).url) end end end diff --git a/spec/helpers/user_notifications_helper_spec.rb b/spec/helpers/user_notifications_helper_spec.rb index 24c1d20935e..a0d266e2b10 100644 --- a/spec/helpers/user_notifications_helper_spec.rb +++ b/spec/helpers/user_notifications_helper_spec.rb @@ -123,6 +123,7 @@ describe UserNotificationsHelper do describe 'when cdn path is configured' do before do SiteSetting.s3_cdn_url = 'https://some.cdn.com' + end it 'should return the right url' do diff --git a/spec/models/admin_dashboard_problem_spec.rb b/spec/models/admin_dashboard_problem_spec.rb index 5629eeb11b5..545bda9c80d 100644 --- a/spec/models/admin_dashboard_problem_spec.rb +++ b/spec/models/admin_dashboard_problem_spec.rb @@ -194,52 +194,6 @@ describe AdminDashboardData do end end - describe 'pwa_config_check' do - subject { described_class.new.pwa_config_check } - - it 'alerts for large_icon missing' do - SiteSetting.large_icon = nil - expect(subject).to eq(I18n.t('dashboard.pwa_config_icon_warning', base_path: Discourse.base_path)) - end - - it 'alerts for incompatible large_icon' do - upload = UploadCreator.new( - file_from_fixtures('large_icon_incorrect.png'), - 'large_icon', - for_site_setting: true - ).create_for(Discourse.system_user.id) - SiteSetting.large_icon = upload - expect(subject).to eq(I18n.t('dashboard.pwa_config_icon_warning', base_path: Discourse.base_path)) - end - - context 'when large_icon is correct' do - before do - upload = UploadCreator.new( - file_from_fixtures('large_icon_correct.png'), - 'large_icon', - for_site_setting: true - ).create_for(Discourse.system_user.id) - SiteSetting.large_icon = upload - end - - it 'alerts for short_title missing' do - SiteSetting.short_title = nil - expect(subject).to eq(I18n.t('dashboard.pwa_config_title_warning', base_path: Discourse.base_path)) - end - - it 'returns nil when everything is ok' do - upload = UploadCreator.new( - file_from_fixtures('large_icon_correct.png'), - 'large_icon', - for_site_setting: true - ).create_for(Discourse.system_user.id) - SiteSetting.large_icon = upload - SiteSetting.short_title = 'title' - expect(subject).to be_nil - end - end - end - describe 's3_config_check' do shared_examples 'problem detection for s3-dependent setting' do subject { described_class.new.s3_config_check } diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 4dcdabefcd6..8a68d638b3f 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -169,6 +169,7 @@ RSpec.configure do |config| SearchIndexer.disable UserActionManager.disable NotificationEmailer.disable + SiteIconManager.disable SiteSetting.provider.all.each do |setting| SiteSetting.remove_override!(setting.name) diff --git a/spec/requests/metadata_controller_spec.rb b/spec/requests/metadata_controller_spec.rb index e387b84bb5d..4e5b9d720c7 100644 --- a/spec/requests/metadata_controller_spec.rb +++ b/spec/requests/metadata_controller_spec.rb @@ -3,13 +3,19 @@ require 'rails_helper' RSpec.describe MetadataController do - let(:upload) { Fabricate(:upload) } - describe 'manifest.webmanifest' do + before do + SiteIconManager.enable + end + + let(:upload) do + UploadCreator.new(file_from_fixtures("smallest.png"), 'logo.png').create_for(Discourse.system_user.id) + end + it 'returns the right output' do title = 'MyApp' SiteSetting.title = title - SiteSetting.large_icon = upload + SiteSetting.manifest_icon = upload get "/manifest.webmanifest" expect(response.status).to eq(200) @@ -19,17 +25,14 @@ RSpec.describe MetadataController do expect(manifest["name"]).to eq(title) expect(manifest["icons"].first["src"]).to eq( - UrlHelper.absolute(upload.url) + UrlHelper.absolute(SiteSetting.site_manifest_icon_url) ) end it 'can guess mime types' do - upload = Fabricate(:upload, - original_filename: 'test.jpg', - extension: 'jpg' - ) + upload = UploadCreator.new(file_from_fixtures("logo.jpg"), 'logo.jpg').create_for(Discourse.system_user.id) - SiteSetting.large_icon = upload + SiteSetting.manifest_icon = upload get "/manifest.webmanifest" expect(response.status).to eq(200) @@ -38,7 +41,7 @@ RSpec.describe MetadataController do end it 'defaults to png' do - SiteSetting.large_icon = upload + SiteSetting.manifest_icon = upload get "/manifest.webmanifest" expect(response.status).to eq(200) manifest = JSON.parse(response.body) @@ -81,6 +84,8 @@ RSpec.describe MetadataController do end describe 'opensearch.xml' do + let(:upload) { Fabricate(:upload) } + it 'returns the right output' do title = 'MyApp' SiteSetting.title = title diff --git a/spec/requests/static_controller_spec.rb b/spec/requests/static_controller_spec.rb index d234dd9cd2a..04b7b08ae69 100644 --- a/spec/requests/static_controller_spec.rb +++ b/spec/requests/static_controller_spec.rb @@ -23,7 +23,7 @@ describe StaticController do expect(response.status).to eq(200) expect(response.content_type).to eq('image/png') - expect(response.body.bytesize).to eq(SiteSetting.favicon.filesize) + expect(response.body.bytesize).to eq(SiteIconManager.favicon.filesize) end it 'returns the configured favicon' do diff --git a/test/javascripts/acceptance/admin-site-settings-test.js.es6 b/test/javascripts/acceptance/admin-site-settings-test.js.es6 index 86fcf962d0d..44ed23aec0b 100644 --- a/test/javascripts/acceptance/admin-site-settings-test.js.es6 +++ b/test/javascripts/acceptance/admin-site-settings-test.js.es6 @@ -1,12 +1,31 @@ import { acceptance } from "helpers/qunit-helpers"; +import { default as siteSettingFixture } from "fixtures/site_settings"; + +var titleOverride = undefined; acceptance("Admin - Site Settings", { loggedIn: true, + beforeEach() { + titleOverride = undefined; + }, pretend(server, helper) { - server.put("/admin/site_settings/**", () => - helper.response({ success: "OK" }) - ); + server.put("/admin/site_settings/title", body => { + titleOverride = body.requestBody.split("=")[1]; + return helper.response({ success: "OK" }); + }); + server.get("/admin/site_settings", () => { + const fixtures = siteSettingFixture["/admin/site_settings"].site_settings; + const titleSetting = Object.assign({}, fixtures[0]); + + if (titleOverride) { + titleSetting.value = titleOverride; + } + const response = { + site_settings: [titleSetting, ...fixtures.slice(1)] + }; + return helper.response(response); + }); } }); diff --git a/test/javascripts/components/image-uploader-test.js.es6 b/test/javascripts/components/image-uploader-test.js.es6 index 684339ec61e..409b5432052 100644 --- a/test/javascripts/components/image-uploader-test.js.es6 +++ b/test/javascripts/components/image-uploader-test.js.es6 @@ -2,7 +2,8 @@ import componentTest from "helpers/component-test"; moduleForComponent("image-uploader", { integration: true }); componentTest("with image", { - template: "{{image-uploader imageUrl='/some/upload.png'}}", + template: + "{{image-uploader imageUrl='/some/upload.png' placeholderUrl='/not/used.png'}}", async test(assert) { assert.equal( @@ -17,6 +18,12 @@ componentTest("with image", { "it displays the trash icon" ); + assert.equal( + find(".placeholder-overlay").length, + 0, + "it does not display the placeholder image" + ); + await click(".image-uploader-lightbox-btn"); assert.equal( @@ -50,3 +57,33 @@ componentTest("without image", { ); } }); + +componentTest("with placeholder", { + template: "{{image-uploader placeholderUrl='/some/image.png'}}", + + test(assert) { + assert.equal( + find(".d-icon-far-image").length, + 1, + "it displays the upload icon" + ); + + assert.equal( + find(".d-icon-far-trash-alt").length, + 0, + "it does not display trash icon" + ); + + assert.equal( + find(".image-uploader-lightbox-btn").length, + 0, + "it does not display the button to open image lightbox" + ); + + assert.equal( + find(".placeholder-overlay").length, + 1, + "it displays the placeholder image" + ); + } +});