From c4d3aa3d471c4e0b810f3d62f7d1b8808fcab0ab Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 2 May 2014 17:46:03 -0400 Subject: [PATCH] Theming: a UI to choose some base colors that are applied to all the site css. CSS compiled outside of asset pipeline. --- .../admin/components/color_input_component.js | 8 +- .../admin_customize_colors_controller.js | 32 ++----- .../javascripts/admin/models/color_scheme.js | 12 ++- .../admin/models/color_scheme_color.js | 34 +++---- .../admin/routes/admin_customize_route.js | 2 +- .../templates/customize_colors.js.handlebars | 20 ++-- .../initializers/live_development.js | 3 +- .../stylesheets/common/admin/admin_base.scss | 11 ++- .../stylesheets/common/base/discourse.scss | 18 ++-- .../stylesheets/common/foundation/colors.scss | 16 ++-- .../common/foundation/variables.scss | 5 +- app/assets/stylesheets/desktop.scss | 15 ++- app/assets/stylesheets/desktop/header.scss | 8 +- .../stylesheets/desktop/topic-post.scss | 8 +- app/assets/stylesheets/mobile.scss | 13 ++- app/assets/stylesheets/mobile/header.scss | 8 +- .../admin/color_schemes_controller.rb | 17 +++- app/models/color_scheme.rb | 41 ++++++-- app/models/color_scheme_color.rb | 96 +------------------ app/models/site_customization.rb | 34 +------ .../color_scheme_color_serializer.rb | 2 +- app/serializers/color_scheme_serializer.rb | 7 +- app/services/color_scheme_revisor.rb | 6 +- .../common/_discourse_stylesheet.html.erb | 6 +- config/initializers/sprockets.rb | 4 +- config/locales/client.en.yml | 9 ++ config/locales/server.en.yml | 4 + db/fixtures/701_color_schemes.rb | 14 --- ...20140506200235_remove_seed_color_scheme.rb | 9 ++ ...remove_opacity_from_color_scheme_colors.rb | 9 ++ lib/autospec/reload_css.rb | 5 + lib/sass/discourse_safe_sass_importer.rb | 32 +++++++ lib/sass/discourse_sass_compiler.rb | 55 +++++++++++ lib/{ => sass}/discourse_sass_importer.rb | 47 ++++++--- lib/sass/discourse_stylesheets.rb | 95 ++++++++++++++++++ lib/tasks/assets.rake | 14 ++- .../discourse_sass_compiler_spec.rb | 30 ++++++ spec/components/discourse_stylesheets_spec.rb | 32 +++++++ .../admin/color_schemes_controller_spec.rb | 22 ++++- .../color_scheme_color_fabricator.rb | 3 +- spec/fixtures/scss/broken.scss | 1 + spec/fixtures/scss/my_plugin.scss | 3 + spec/models/color_scheme_color_spec.rb | 18 ++++ spec/models/color_scheme_spec.rb | 26 ++++- spec/models/site_customization_spec.rb | 4 +- spec/services/color_scheme_revisor_spec.rb | 48 ++++++---- 46 files changed, 596 insertions(+), 310 deletions(-) delete mode 100644 db/fixtures/701_color_schemes.rb create mode 100644 db/migrate/20140506200235_remove_seed_color_scheme.rb create mode 100644 db/migrate/20140507173327_remove_opacity_from_color_scheme_colors.rb create mode 100644 lib/sass/discourse_safe_sass_importer.rb create mode 100644 lib/sass/discourse_sass_compiler.rb rename lib/{ => sass}/discourse_sass_importer.rb (73%) create mode 100644 lib/sass/discourse_stylesheets.rb create mode 100644 spec/components/discourse_sass_compiler_spec.rb create mode 100644 spec/components/discourse_stylesheets_spec.rb create mode 100644 spec/fixtures/scss/broken.scss create mode 100644 spec/fixtures/scss/my_plugin.scss create mode 100644 spec/models/color_scheme_color_spec.rb diff --git a/app/assets/javascripts/admin/components/color_input_component.js b/app/assets/javascripts/admin/components/color_input_component.js index b1626dc46..099d4ab1e 100644 --- a/app/assets/javascripts/admin/components/color_input_component.js +++ b/app/assets/javascripts/admin/components/color_input_component.js @@ -2,6 +2,8 @@ An input field for a color. @param hexValue is a reference to the color's hex value. + @param brightnessValue is a number from 0 to 255 representing the brightness of the color. See ColorSchemeColor. + @params valid is a boolean indicating if the input field is a valid color. @class Discourse.ColorInputComponent @extends Ember.Component @@ -13,10 +15,12 @@ Discourse.ColorInputComponent = Ember.Component.extend({ hexValueChanged: function() { var hex = this.get('hexValue'); - if (hex && (hex.length === 3 || hex.length === 6) && this.get('brightnessValue')) { + if (this.get('valid')) { this.$('input').attr('style', 'color: ' + (this.get('brightnessValue') > 125 ? 'black' : 'white') + '; background-color: #' + hex + ';'); + } else { + this.$('input').attr('style', ''); } - }.observes('hexValue', 'brightnessValue'), + }.observes('hexValue', 'brightnessValue', 'valid'), didInsertElement: function() { var self = this; diff --git a/app/assets/javascripts/admin/controllers/admin_customize_colors_controller.js b/app/assets/javascripts/admin/controllers/admin_customize_colors_controller.js index db9d3d8ba..e42c3e116 100644 --- a/app/assets/javascripts/admin/controllers/admin_customize_colors_controller.js +++ b/app/assets/javascripts/admin/controllers/admin_customize_colors_controller.js @@ -8,11 +8,10 @@ **/ Discourse.AdminCustomizeColorsController = Ember.ArrayController.extend({ - filter: null, onlyOverridden: false, baseColorScheme: function() { - return this.get('model').findBy('id', 1); + return this.get('model').findBy('is_base', true); }.property('model.@each.id'), baseColors: function() { @@ -28,15 +27,10 @@ Discourse.AdminCustomizeColorsController = Ember.ArrayController.extend({ this.set('selectedItem', null); }, - filterContent: Discourse.debounce(function() { + filterContent: function() { if (!this.get('selectedItem')) { return; } - var filter; - if (this.get('filter')) { - filter = this.get('filter').toLowerCase(); - } - - if ((filter === undefined || filter.length < 1) && !this.get('onlyOverridden')) { + if (!this.get('onlyOverridden')) { this.set('colors', this.get('selectedItem.colors')); return; } @@ -44,18 +38,12 @@ Discourse.AdminCustomizeColorsController = Ember.ArrayController.extend({ var matches = Em.A(), self = this, baseColor; _.each(this.get('selectedItem.colors'), function(color){ - if (filter === undefined || filter.length < 1 || color.get('name').toLowerCase().indexOf(filter) > -1) { - if (self.get('onlyOverridden')) { - baseColor = self.get('baseColors').get(color.get('name')); - if (color.get('hex') === baseColor.get('hex') && color.get('opacity') === baseColor.get('opacity')) { - return; - } - } - matches.pushObject(color); - } + baseColor = self.get('baseColors').get(color.get('name')); + if (color.get('hex') !== baseColor.get('hex')) matches.pushObject(color); }); + this.set('colors', matches); - }, 250).observes('filter', 'onlyOverridden'), + }.observes('onlyOverridden'), actions: { selectColorScheme: function(colorScheme) { @@ -64,6 +52,7 @@ Discourse.AdminCustomizeColorsController = Ember.ArrayController.extend({ this.set('colors', colorScheme.get('colors')); colorScheme.set('savingStatus', null); colorScheme.set('selected', true); + this.filterContent(); }, newColorScheme: function() { @@ -71,10 +60,7 @@ Discourse.AdminCustomizeColorsController = Ember.ArrayController.extend({ newColorScheme.set('name', I18n.t('admin.customize.colors.new_name')); this.pushObject(newColorScheme); this.send('selectColorScheme', newColorScheme); - }, - - clearFilter: function() { - this.set('filter', null); + this.set('onlyOverridden', false); }, undo: function(color) { diff --git a/app/assets/javascripts/admin/models/color_scheme.js b/app/assets/javascripts/admin/models/color_scheme.js index 053cad227..43315bd6d 100644 --- a/app/assets/javascripts/admin/models/color_scheme.js +++ b/app/assets/javascripts/admin/models/color_scheme.js @@ -27,7 +27,7 @@ Discourse.ColorScheme = Discourse.Model.extend(Ember.Copyable, { copy: function() { var newScheme = Discourse.ColorScheme.create({name: this.get('name'), enabled: false, can_edit: true, colors: Em.A()}); _.each(this.get('colors'), function(c){ - newScheme.colors.pushObject(Discourse.ColorSchemeColor.create({name: c.get('name'), hex: c.get('hex'), opacity: c.get('opacity')})); + newScheme.colors.pushObject(Discourse.ColorSchemeColor.create({name: c.get('name'), hex: c.get('hex')})); }); return newScheme; }, @@ -40,7 +40,7 @@ Discourse.ColorScheme = Discourse.Model.extend(Ember.Copyable, { }.property('name', 'enabled', 'colors.@each.changed', 'saving'), disableSave: function() { - return !this.get('changed') || this.get('saving'); + return !this.get('changed') || this.get('saving') || _.any(this.get('colors'), function(c) { return !c.get('valid'); }); }.property('changed'), newRecord: function() { @@ -48,6 +48,8 @@ Discourse.ColorScheme = Discourse.Model.extend(Ember.Copyable, { }.property('id'), save: function() { + if (this.get('is_base') || this.get('disableSave')) return; + var self = this; this.set('savingStatus', I18n.t('saving')); this.set('saving',true); @@ -57,7 +59,7 @@ Discourse.ColorScheme = Discourse.Model.extend(Ember.Copyable, { data.colors = []; _.each(this.get('colors'), function(c) { if (!self.id || c.get('changed')) { - data.colors.pushObject({name: c.get('name'), hex: c.get('hex'), opacity: c.get('opacity')}); + data.colors.pushObject({name: c.get('name'), hex: c.get('hex')}); } }); @@ -104,8 +106,8 @@ Discourse.ColorScheme.reopenClass({ id: colorScheme.id, name: colorScheme.name, enabled: colorScheme.enabled, - can_edit: colorScheme.can_edit, - colors: colorScheme.colors.map(function(c) { return Discourse.ColorSchemeColor.create({name: c.name, hex: c.hex, opacity: c.opacity}); }) + is_base: colorScheme.is_base, + colors: colorScheme.colors.map(function(c) { return Discourse.ColorSchemeColor.create({name: c.name, hex: c.hex}); }) })); }); colorSchemes.set('loading', false); diff --git a/app/assets/javascripts/admin/models/color_scheme_color.js b/app/assets/javascripts/admin/models/color_scheme_color.js index 9c6420a75..32e908553 100644 --- a/app/assets/javascripts/admin/models/color_scheme_color.js +++ b/app/assets/javascripts/admin/models/color_scheme_color.js @@ -15,30 +15,24 @@ Discourse.ColorSchemeColor = Discourse.Model.extend({ }, startTrackingChanges: function() { - this.set('originals', { - hex: this.get('hex') || 'FFFFFF', - opacity: this.get('opacity') || '100' - }); + this.set('originals', {hex: this.get('hex') || 'FFFFFF'}); this.notifyPropertyChange('hex'); // force changed property to be recalculated }, changed: function() { if (!this.originals) return false; - - if (this.get('hex') !== this.originals['hex'] || this.get('opacity').toString() !== this.originals['opacity'].toString()) { - return true; - } else { - return false; - } - }.property('hex', 'opacity'), + if (this.get('hex') !== this.originals['hex']) return true; + return false; + }.property('hex'), undo: function() { - if (this.originals) { - this.set('hex', this.originals['hex']); - this.set('opacity', this.originals['opacity']); - } + if (this.originals) this.set('hex', this.originals['hex']); }, + translatedName: function() { + return I18n.t('admin.customize.colors.' + this.get('name')); + }.property('name'), + /** brightness returns a number between 0 (darkest) to 255 (brightest). Undefined if hex is not a valid color. @@ -61,11 +55,7 @@ Discourse.ColorSchemeColor = Discourse.Model.extend({ } }.observes('hex'), - opacityChanged: function() { - if (this.get('opacity')) { - var o = this.get('opacity').toString().replace(/[^\d.]/g, ""); - if (parseInt(o,10) > 100) { o = o.substr(0,o.length-1); } - this.set('opacity', o); - } - }.observes('opacity') + valid: function() { + return this.get('hex').match(/^([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/) !== null; + }.property('hex') }); diff --git a/app/assets/javascripts/admin/routes/admin_customize_route.js b/app/assets/javascripts/admin/routes/admin_customize_route.js index d7eca41d8..701a03fe5 100644 --- a/app/assets/javascripts/admin/routes/admin_customize_route.js +++ b/app/assets/javascripts/admin/routes/admin_customize_route.js @@ -8,6 +8,6 @@ **/ Discourse.AdminCustomizeIndexRoute = Discourse.Route.extend({ redirect: function() { - this.transitionTo('adminCustomize.css_html'); + this.transitionTo('adminCustomize.colors'); } }); \ No newline at end of file diff --git a/app/assets/javascripts/admin/templates/customize_colors.js.handlebars b/app/assets/javascripts/admin/templates/customize_colors.js.handlebars index ea8493627..af7840432 100644 --- a/app/assets/javascripts/admin/templates/customize_colors.js.handlebars +++ b/app/assets/javascripts/admin/templates/customize_colors.js.handlebars @@ -4,9 +4,9 @@

{{i18n admin.customize.colors.long_title}}

@@ -36,27 +36,22 @@ {{i18n admin.site_settings.show_overriden}} -
- {{textField value=filter placeholderKey="type_to_filter"}} - -
+ {{#if colors.length}} - {{#each colors}} - - - - + + + @@ -64,6 +59,9 @@ {{/each}}
{{i18n admin.customize.color}}{{i18n admin.customize.opacity}}
{{name}}{{color-input hexValue=hex brightnessValue=brightness}}{{textField class="opacity-input" value=opacity maxlength="3"}}
{{translatedName}}{{color-input hexValue=hex brightnessValue=brightness valid=valid}}
+ {{else}} +

{{i18n search.no_results}}

+ {{/if}} {{else}} diff --git a/app/assets/javascripts/discourse/initializers/live_development.js b/app/assets/javascripts/discourse/initializers/live_development.js index 322f9f166..69a6b4edd 100644 --- a/app/assets/javascripts/discourse/initializers/live_development.js +++ b/app/assets/javascripts/discourse/initializers/live_development.js @@ -62,7 +62,8 @@ Discourse.addInitializer(function() { if (!$(this).data('orig')) { $(this).data('orig', this.href); } - this.href = $(this).data('orig') + "&hash=" + me.hash; + var orig = $(this).data('orig'); + this.href = orig + (orig.indexOf('?') >= 0 ? "&hash=" : "?hash=") + me.hash; } }); } diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index ba784dd7c..ebea6c63c 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -4,6 +4,7 @@ @import "common/foundation/helpers"; + .admin-contents table { width: 100%; tr {text-align: left;} @@ -434,14 +435,18 @@ section.details { .colors { thead th { border: none; } td.hex { width: 100px; } + td.changed { width: 300px; } .hex-input { width: 80px; } - td.opacity { width: 50px; } - .opacity-input { width: 30px; } - .hex, .opacity { text-align: center; } + .hex { text-align: center; } .changed .name { color: scale-color($highlight, $lightness: -50%); } + .invalid .hex input { + background-color: white; + color: black; + border-color: $danger; + } } } diff --git a/app/assets/stylesheets/common/base/discourse.scss b/app/assets/stylesheets/common/base/discourse.scss index 986860512..028aa7f50 100644 --- a/app/assets/stylesheets/common/base/discourse.scss +++ b/app/assets/stylesheets/common/base/discourse.scss @@ -77,18 +77,18 @@ body { .clear-transitions { @include transition(none !important); } - form { - .tip { - display: inline-block; - &.good { - color: $success; - } - &.bad { - color: $danger; - } + +.tip { + display: inline-block; + &.good { + color: $success; + } + &.bad { + color: $danger; } } + #wmd-input { resize: none; } diff --git a/app/assets/stylesheets/common/foundation/colors.scss b/app/assets/stylesheets/common/foundation/colors.scss index 99b06d19d..91f877eda 100644 --- a/app/assets/stylesheets/common/foundation/colors.scss +++ b/app/assets/stylesheets/common/foundation/colors.scss @@ -1,7 +1,9 @@ -$primary: #333333 !default; -$secondary: #ffffff !default; -$tertiary: #0088cc !default; -$highlight: #ffff4d !default; -$danger: #e45735 !default; -$success: #009900 !default; -$love: #fa6c8d !default; +$primary: #333333 !default; +$secondary: #ffffff !default; +$tertiary: #0088cc !default; +$header_background: #ffffff !default; +$header_primary: #333333 !default; +$highlight: #ffff4d !default; +$danger: #e45735 !default; +$success: #009900 !default; +$love: #fa6c8d !default; diff --git a/app/assets/stylesheets/common/foundation/variables.scss b/app/assets/stylesheets/common/foundation/variables.scss index f81d897e1..518e38615 100644 --- a/app/assets/stylesheets/common/foundation/variables.scss +++ b/app/assets/stylesheets/common/foundation/variables.scss @@ -26,7 +26,6 @@ $base-font-size: 14px !default; $base-line-height: 19px !default; $base-font-family: Helvetica, Arial, sans-serif !default; -@import "colors"; - -/* This file doesn't actually exist, it is injected by DiscourseSassImporter. */ +/* These files don't actually exist. They're injected by DiscourseSassImporter. */ +@import "theme_variables"; @import "plugins_variables"; diff --git a/app/assets/stylesheets/desktop.scss b/app/assets/stylesheets/desktop.scss index 8fe56240a..f6f3a00c8 100644 --- a/app/assets/stylesheets/desktop.scss +++ b/app/assets/stylesheets/desktop.scss @@ -1,5 +1,18 @@ @import "common"; -@import "desktop/*"; + +/* @import "desktop/*"; I don't know why this doesn't work. */ + +@import "desktop/compose"; +@import "desktop/discourse"; +@import "desktop/header"; +@import "desktop/login"; +@import "desktop/modal"; +@import "desktop/poster_expansion"; +@import "desktop/topic-list"; +@import "desktop/topic-post"; +@import "desktop/topic"; +@import "desktop/upload"; +@import "desktop/user"; /* These files doesn't actually exist, they are injected by DiscourseSassImporter. */ diff --git a/app/assets/stylesheets/desktop/header.scss b/app/assets/stylesheets/desktop/header.scss index fe9f60e88..37131a81a 100644 --- a/app/assets/stylesheets/desktop/header.scss +++ b/app/assets/stylesheets/desktop/header.scss @@ -8,8 +8,8 @@ top: 0; left: 0; z-index: 1000; - box-shadow: 0 2px 4px -2px rgba($primary, .25); - background-color: $secondary; + box-shadow: 0 2px 4px -2px rgba($header_primary, .25); + background-color: $header_background; padding-top: 3px; .docked & { position: fixed; @@ -42,7 +42,7 @@ .current-username { float: left; a { - color: $primary; + color: $header_primary; font-size: 14px; display:block; margin-top: 10px; @@ -65,7 +65,7 @@ .icon { display: block; padding: 3px; - color: scale-color($primary, $lightness: 50%); + color: scale-color($header_primary, $lightness: 50%); text-decoration: none; cursor: pointer; border-top: 1px solid transparent; diff --git a/app/assets/stylesheets/desktop/topic-post.scss b/app/assets/stylesheets/desktop/topic-post.scss index 006da6caf..5928a4c98 100644 --- a/app/assets/stylesheets/desktop/topic-post.scss +++ b/app/assets/stylesheets/desktop/topic-post.scss @@ -528,12 +528,10 @@ iframe { width: 78%; max-width: 800px; .topic-statuses { - i { color: $primary; -} - .unpinned { color: $primary;} - + i { color: $header_primary; } + .unpinned { color: $header_primary; } } - .topic-link {color: $primary;} + .topic-link { color: $header_primary; } } diff --git a/app/assets/stylesheets/mobile.scss b/app/assets/stylesheets/mobile.scss index 063f58aab..50eaedc6f 100644 --- a/app/assets/stylesheets/mobile.scss +++ b/app/assets/stylesheets/mobile.scss @@ -1,6 +1,17 @@ @import "common"; -@import "mobile/*"; +/* @import "mobile/*"; I don't know why this doesn't work */ +@import "mobile/compose"; +@import "mobile/discourse"; +@import "mobile/faqs"; +@import "mobile/header"; +@import "mobile/login"; +@import "mobile/modal"; +@import "mobile/topic-list"; +@import "mobile/topic-post"; +@import "mobile/topic"; +@import "mobile/upload"; +@import "mobile/user"; /* These files doesn't actually exist, they are injected by DiscourseSassImporter. */ diff --git a/app/assets/stylesheets/mobile/header.scss b/app/assets/stylesheets/mobile/header.scss index 83271b3d6..0dc4c6a5d 100644 --- a/app/assets/stylesheets/mobile/header.scss +++ b/app/assets/stylesheets/mobile/header.scss @@ -9,8 +9,8 @@ position: absolute; top: 0; z-index: 1001; - background-color: $secondary; - box-shadow: 0 0 3px rgba($primary, .6); + background-color: $header_background; + box-shadow: 0 0 3px rgba($header_primary, .6); .docked & { position: fixed; @@ -47,7 +47,7 @@ float: left; display: none; a { - color: darken($primary, 40%); + color: darken($header_primary, 40%); font-size: 14px; line-height: 40px; } @@ -69,7 +69,7 @@ .icon { display: block; padding: 4px; - color: scale-color($primary, $lightness: 50%); + color: scale-color($header_primary, $lightness: 50%); text-decoration: none; cursor: pointer; &:hover { diff --git a/app/controllers/admin/color_schemes_controller.rb b/app/controllers/admin/color_schemes_controller.rb index 569391bb5..35f45c6e4 100644 --- a/app/controllers/admin/color_schemes_controller.rb +++ b/app/controllers/admin/color_schemes_controller.rb @@ -3,16 +3,25 @@ class Admin::ColorSchemesController < Admin::AdminController before_filter :fetch_color_scheme, only: [:update, :destroy] def index - render_serialized(ColorScheme.current_version.order('id ASC').all.to_a, ColorSchemeSerializer) + render_serialized([ColorScheme.base] + ColorScheme.current_version.order('id ASC').all.to_a, ColorSchemeSerializer) end def create color_scheme = ColorScheme.create(color_scheme_params) - render json: color_scheme, root: false + if color_scheme.valid? + render json: color_scheme, root: false + else + render_json_error(color_scheme) + end end def update - render json: ColorSchemeRevisor.revise(@color_scheme, color_scheme_params), root: false + color_scheme = ColorSchemeRevisor.revise(@color_scheme, color_scheme_params) + if color_scheme.valid? + render json: color_scheme, root: false + else + render_json_error(color_scheme) + end end def destroy @@ -28,6 +37,6 @@ class Admin::ColorSchemesController < Admin::AdminController end def color_scheme_params - params.permit(color_scheme: [:enabled, :name, colors: [:name, :hex, :opacity]])[:color_scheme] + params.permit(color_scheme: [:enabled, :name, colors: [:name, :hex]])[:color_scheme] end end diff --git a/app/models/color_scheme.rb b/app/models/color_scheme.rb index 34731dafb..5a5813e9c 100644 --- a/app/models/color_scheme.rb +++ b/app/models/color_scheme.rb @@ -1,5 +1,7 @@ class ColorScheme < ActiveRecord::Base + attr_accessor :is_base + has_many :color_scheme_colors, -> { order('id ASC') }, dependent: :destroy alias_method :colors, :color_scheme_colors @@ -8,22 +10,41 @@ class ColorScheme < ActiveRecord::Base after_destroy :destroy_versions - def self.enabled - current_version.find_by(enabled: true) || find(1) + validates_associated :color_scheme_colors + + BASE_COLORS_FILE = "#{Rails.root}/app/assets/stylesheets/common/foundation/colors.scss" + + @mutex = Mutex.new + + def self.base_colors + @mutex.synchronize do + return @base_colors if @base_colors + @base_colors = {} + File.readlines(BASE_COLORS_FILE).each do |line| + matches = /\$([\w]+):\s*#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})(?:[;]|\s)/.match(line.strip) + @base_colors[matches[1]] = matches[2] if matches + end + end + @base_colors end - def can_edit? - self.id != 1 # base theme shouldn't be edited, except by seed data + def self.enabled + current_version.find_by(enabled: true) end + def self.base + return @base_color if @base_color + @base_color = new(name: I18n.t('color_schemes.base_theme_name'), enabled: false) + @base_color.colors = base_colors.map { |name, hex| {name: name, hex: hex} } + @base_color.is_base = true + @base_color + end + + def colors=(arr) @colors_by_name = nil arr.each do |c| - self.color_scheme_colors << ColorSchemeColor.new( - name: c[:name], - hex: c[:hex], - opacity: c[:opacity].to_i - ) + self.color_scheme_colors << ColorSchemeColor.new( name: c[:name], hex: c[:hex] ) end end @@ -36,7 +57,7 @@ class ColorScheme < ActiveRecord::Base def colors_hashes color_scheme_colors.map do |c| - {name: c.name, hex: c.hex, opacity: c.opacity} + {name: c.name, hex: c.hex} end end diff --git a/app/models/color_scheme_color.rb b/app/models/color_scheme_color.rb index fe5f0b628..058d33b6c 100644 --- a/app/models/color_scheme_color.rb +++ b/app/models/color_scheme_color.rb @@ -1,101 +1,7 @@ class ColorSchemeColor < ActiveRecord::Base belongs_to :color_scheme - BASE_COLORS = { - primary_border_color: "e6e6e6", - secondary_border_color: "f5f5f5", - tertiary_border_color: "ffffff", - highlight_border_color: "ffff4d", - emphasis_border_color: "00aaff", - warning_border_color: "f0a28f", - success_border_color: "009900", - primary_background_color: "ffffff", - secondary_background_color: "333333", - tertiary_background_color: "4d4d4d", - moderator_background_color: "ffffe5", - emphasis_background_color: "e5f6ff", - success_background_color: "99ff99", - warning_background_color: "f6c7bc", - highlight_background_color: "ffffc2", - like_background_color: "fee7ed", - composer_background_color: "e6e6e6", - notification_badge_background_color: "8c8c8c", - primary_text_color: "333333", - secondary_text_color: "999999", - tertiary_text_color: "ffffff", - warning_text_color: "e45735", - success_text_color: "009900", - emphasis_text_color: "00aaff", - highlight_text_color: "ffff4d", - like_color: "fa6c8d", - primary_shadow_color: "333333", - secondary_shadow_color: "ffffff", - warning_shadow_color: "f0a28f", - success_shadow_color: "009900", - highlight: "ffff4d", - header_item_highlight: "e5f6ff", - link_color: "0088cc", - secondary_link_color: "ffffff", - "muted-link-color" => "8c8c8c", - "muted-important-link-color" => "8c8c8c", - "link-color-visited" => "0088cc", - "link-color-hover" => "0088cc", - "link-color-active" => "0088cc", - heatmap_high: "ea7c62", - heatmap_med: "e45735", - heatmap_low: "cb3d1b", - coldmap_high: "33bbff", - coldmap_med: "00aaff", - coldmap_low: "0088cc", - "btn-default-color" => "333333", - "btn-default-background-color" => "b3b3b3", - "btn-default-background-color-hover" => "f5f5f5", - "btn-primary-border-color" => "0088cc", - "btn-primary-background-color" => "00aaff", - "btn-primary-background-color-dark" => "00aaff", - "btn-primary-background-color-light" => "99ddff", - "btn-danger-border-color" => "cb3d1b", - "btn-danger-background-color" => "e45735", - "btn-danger-background-color-dark" => "cb3d1b", - "btn-danger-background-color-light" => "e45735", - "btn-success-background" => "00b300", - "nav-pills-color" => "333333", - "nav-pills-color-hover" => "e45735", - "nav-pills-border-color-hover" => "f9dad2", - "nav-pills-background-color-hover" => "f9dad2", - "nav-pills-color-active" => "e45735", - "nav-pills-border-color-active" => "cb3d1b", - "nav-pills-background-color-active" => "e45735", - "nav-stacked-color" => "333333", - "nav-stacked-border-color" => "cccccc", - "nav-stacked-background-color" => "f5f5f5", - "nav-stacked-divider-color" => "cccccc", - "nav-stacked-chevron-color" => "b3b3b3", - "nav-stacked-border-color-active" => "e45735", - "nav-stacked-background-color-active" => "e45735", - "nav-button-color-hover" => "333333", - "nav-button-background-color-hover" => "cccccc", - "nav-button-color-active" => "333333", - "nav-button-background-color-active" => "cccccc", - "modal-header-color" => "e45735", - "modal-header-border-color" => "b3b3b3", - "modal-close-button-color" => "b3b3b3", - "nav-like-button-color-hover" => "fa6c8d", - "nav-like-button-background-color-hover" => "fed9e1", - "nav-like-button-color-active" => "f83b67", - "nav-like-button-background-color-active" => "fed9e1", - "topic-list-border-color" => "b3b3b3", - "topic-list-th-color" => "8c8c8c", - "topic-list-th-border-color" => "b3b3b3", - "topic-list-th-background-color" => "f5f5f5", - "topic-list-td-color" => "8c8c8c", - "topic-list-td-border-color" => "cccccc", - "topic-list-star-color" => "cccccc", - "topic-list-starred-color" => "e45735", - "quote-background" => "f5f5f5", - topicMenuColor: "333333", - bookmarkColor: "00aaff" - } + validates :hex, format: { with: /\A([0-9a-fA-F]{3}|[0-9a-fA-F]{6})\z/ } end # == Schema Information diff --git a/app/models/site_customization.rb b/app/models/site_customization.rb index eac472b80..de2f7e970 100644 --- a/app/models/site_customization.rb +++ b/app/models/site_customization.rb @@ -1,4 +1,4 @@ -require_dependency 'discourse_sass_importer' +require_dependency 'sass/discourse_sass_compiler' class SiteCustomization < ActiveRecord::Base ENABLED_KEY = '7e202ef2-56d7-47d5-98d8-a9c8d15e57dd' @@ -14,29 +14,7 @@ class SiteCustomization < ActiveRecord::Base end def compile_stylesheet(scss) - env = Rails.application.assets - - # In production Rails.application.assets is a Sprockets::Index - # instead of Sprockets::Environment, there is no cleaner way - # to get the environment from the index. - if env.is_a?(Sprockets::Index) - env = env.instance_variable_get('@environment') - end - - context = env.context_class.new(env, "custom.scss", "app/assets/stylesheets/custom.scss") - - ::Sass::Engine.new(scss, { - syntax: :scss, - cache: false, - read_cache: false, - style: :compressed, - filesystem_importer: DiscourseSassImporter, - sprockets: { - context: context, - environment: context.environment - } - }).render - + DiscourseSassCompiler.compile(scss, 'custom') rescue => e puts e.backtrace.join("\n") unless Sass::SyntaxError === e @@ -49,13 +27,7 @@ class SiteCustomization < ActiveRecord::Base begin self.send("#{stylesheet_attr}_baked=", compile_stylesheet(self.send(stylesheet_attr))) rescue Sass::SyntaxError => e - error = e.sass_backtrace_str("custom stylesheet") - error.gsub!("\n", '\A ') - error.gsub!("'", '\27 ') - - self.send("#{stylesheet_attr}_baked=", - "footer { white-space: pre; } - footer:after { content: '#{error}' }") + self.send("#{stylesheet_attr}_baked=", DiscourseSassCompiler.error_as_css(e, "custom stylesheet")) end end end diff --git a/app/serializers/color_scheme_color_serializer.rb b/app/serializers/color_scheme_color_serializer.rb index 53544688f..c3d4460af 100644 --- a/app/serializers/color_scheme_color_serializer.rb +++ b/app/serializers/color_scheme_color_serializer.rb @@ -1,5 +1,5 @@ class ColorSchemeColorSerializer < ApplicationSerializer - attributes :name, :hex, :opacity + attributes :name, :hex def hex object.hex # otherwise something crazy is returned diff --git a/app/serializers/color_scheme_serializer.rb b/app/serializers/color_scheme_serializer.rb index 75394fa5e..bf09ff401 100644 --- a/app/serializers/color_scheme_serializer.rb +++ b/app/serializers/color_scheme_serializer.rb @@ -1,9 +1,8 @@ class ColorSchemeSerializer < ApplicationSerializer - attributes :id, :name, :enabled, :can_edit - + attributes :id, :name, :enabled, :is_base has_many :colors, serializer: ColorSchemeColorSerializer, embed: :objects - def can_edit - object.can_edit? + def base + object.is_base || false end end \ No newline at end of file diff --git a/app/services/color_scheme_revisor.rb b/app/services/color_scheme_revisor.rb index 32dffcd1d..67cdfa0e0 100644 --- a/app/services/color_scheme_revisor.rb +++ b/app/services/color_scheme_revisor.rb @@ -16,7 +16,7 @@ class ColorSchemeRevisor def revise ColorScheme.transaction do if @params[:enabled] - ColorScheme.update_all enabled: false + ColorScheme.where('id != ?', @color_scheme.id).update_all enabled: false end @color_scheme.name = @params[:name] @@ -25,7 +25,7 @@ class ColorSchemeRevisor if @params[:colors] new_version = @params[:colors].any? do |c| - (existing = @color_scheme.colors_by_name[c[:name]]).nil? or existing.hex != c[:hex] or existing.opacity != c[:opacity] + (existing = @color_scheme.colors_by_name[c[:name]]).nil? or existing.hex != c[:hex] end end @@ -47,7 +47,7 @@ class ColorSchemeRevisor end end - @color_scheme.save! + @color_scheme.save @color_scheme.clear_colors_cache end @color_scheme diff --git a/app/views/common/_discourse_stylesheet.html.erb b/app/views/common/_discourse_stylesheet.html.erb index b7cc1d523..a249e1b45 100644 --- a/app/views/common/_discourse_stylesheet.html.erb +++ b/app/views/common/_discourse_stylesheet.html.erb @@ -1,9 +1,5 @@ <%- unless SiteCustomization.override_default_style(session[:preview_style]) %> - <% if mobile_view? %> - <%= stylesheet_link_tag "mobile" %> - <% else %> - <%= stylesheet_link_tag "desktop" %> - <% end %> + <%= DiscourseStylesheets.stylesheet_link_tag(mobile_view? ? :mobile : :desktop) %> <%- end %> <%- if staff? %> diff --git a/config/initializers/sprockets.rb b/config/initializers/sprockets.rb index b67897963..89ab09986 100644 --- a/config/initializers/sprockets.rb +++ b/config/initializers/sprockets.rb @@ -1,4 +1,6 @@ -require_dependency 'discourse_sass_importer' +require_dependency 'sass/discourse_stylesheets' +require_dependency 'sass/discourse_sass_importer' +require_dependency 'sass/discourse_safe_sass_importer' Sprockets.send(:remove_const, :SassImporter) Sprockets::SassImporter = DiscourseSassImporter diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d4dc2a346..ecdb9c67f 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1466,6 +1466,15 @@ en: copy_name_prefix: "Copy of" delete_confirm: "Delete this color scheme?" under_construction: "NOTE: Changes made here will do nothing! This feature is under construction!" + primary: 'primary' + secondary: 'secondary' + tertiary: 'tertiary' + header_background: "header background" + header_primary: "header primary" + highlight: 'highlight' + danger: 'danger' + success: 'success' + love: 'love' email: diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index abcb1d53d..ccd9a12f0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -211,6 +211,10 @@ en: common: "is one of the 10000 most common passwords. Please use a more secure password." ip_address: signup_not_allowed: "Signup is not allowed from this account." + color_scheme_color: + attributes: + hex: + invalid: "is not a valid color" user_profile: no_info_me: "
the About Me field of your profile is currently blank, would you like to fill it out?
" diff --git a/db/fixtures/701_color_schemes.rb b/db/fixtures/701_color_schemes.rb deleted file mode 100644 index bd0c36605..000000000 --- a/db/fixtures/701_color_schemes.rb +++ /dev/null @@ -1,14 +0,0 @@ -ColorScheme.seed do |s| - s.id = 1 - s.name = I18n.t("color_schemes.base_theme_name") - s.enabled = false -end - -ColorSchemeColor::BASE_COLORS.each_with_index do |color, i| - ColorSchemeColor.seed do |c| - c.id = i+1 - c.name = color[0] - c.hex = color[1] - c.color_scheme_id = 1 - end -end diff --git a/db/migrate/20140506200235_remove_seed_color_scheme.rb b/db/migrate/20140506200235_remove_seed_color_scheme.rb new file mode 100644 index 000000000..b27e9dbc2 --- /dev/null +++ b/db/migrate/20140506200235_remove_seed_color_scheme.rb @@ -0,0 +1,9 @@ +class RemoveSeedColorScheme < ActiveRecord::Migration + def up + execute "DELETE FROM color_schemes WHERE id = 1" + execute "DELETE FROM color_scheme_colors WHERE color_scheme_id = 1" + end + + def down + end +end diff --git a/db/migrate/20140507173327_remove_opacity_from_color_scheme_colors.rb b/db/migrate/20140507173327_remove_opacity_from_color_scheme_colors.rb new file mode 100644 index 000000000..2b4e31e19 --- /dev/null +++ b/db/migrate/20140507173327_remove_opacity_from_color_scheme_colors.rb @@ -0,0 +1,9 @@ +class RemoveOpacityFromColorSchemeColors < ActiveRecord::Migration + def up + remove_column :color_scheme_colors, :opacity + end + + def down + add_column :color_scheme_colors, :opacity, :integer, null: false, default: 100 + end +end diff --git a/lib/autospec/reload_css.rb b/lib/autospec/reload_css.rb index 5c69bdf94..743157d34 100644 --- a/lib/autospec/reload_css.rb +++ b/lib/autospec/reload_css.rb @@ -24,6 +24,11 @@ class Autospec::ReloadCss end def self.run_on_change(paths) + if paths.any? { |p| p =~ /\.(css|s[ac]ss)/ } + s = DiscourseStylesheets.new(:desktop) # TODO: what about mobile? + s.compile + paths << "public" + s.stylesheet_relpath_no_digest + end paths.map! do |p| hash = nil fullpath = "#{Rails.root}/#{p}" diff --git a/lib/sass/discourse_safe_sass_importer.rb b/lib/sass/discourse_safe_sass_importer.rb new file mode 100644 index 000000000..43b6375cb --- /dev/null +++ b/lib/sass/discourse_safe_sass_importer.rb @@ -0,0 +1,32 @@ +require_dependency 'sass/discourse_sass_importer' + +# This custom importer is used to import stylesheets but excludes plugins and theming. +# It's used as a fallback when compilation of stylesheets fails. + +class DiscourseSafeSassImporter < DiscourseSassImporter + def special_imports + super.merge({ + "plugins" => [], + "plugins_mobile" => [], + "plugins_desktop" => [], + "plugins_variables" => [] + }) + end + + def find(name, options) + if name == "theme_variables" + # Load the default variables + contents = "" + special_imports[name].each do |css_file| + contents << File.read(css_file) + end + Sass::Engine.new(contents, options.merge( + filename: "#{name}.scss", + importer: self, + syntax: :scss + )) + else + super(name, options) + end + end +end diff --git a/lib/sass/discourse_sass_compiler.rb b/lib/sass/discourse_sass_compiler.rb new file mode 100644 index 000000000..1a1c4226a --- /dev/null +++ b/lib/sass/discourse_sass_compiler.rb @@ -0,0 +1,55 @@ +require_dependency 'sass/discourse_sass_importer' + +class DiscourseSassCompiler + + def self.compile(scss, target, opts={}) + self.new(scss, target).compile(opts) + end + + # Takes a Sass::SyntaxError and generates css that will show the + # error at the bottom of the page. + def self.error_as_css(sass_error, label) + error = sass_error.sass_backtrace_str(label) + error.gsub!("\n", '\A ') + error.gsub!("'", '\27 ') + + "footer { white-space: pre; } + footer:after { content: '#{error}' }" + end + + + def initialize(scss, target) + @scss = scss + @target = target + end + + # Compiles the given scss and output the css as a string. + # + # Options: + # safe: (boolean) if true, theme and plugin stylesheets will not be included. Default is false. + def compile(opts={}) + env = Rails.application.assets + + # In production Rails.application.assets is a Sprockets::Index + # instead of Sprockets::Environment, there is no cleaner way + # to get the environment from the index. + if env.is_a?(Sprockets::Index) + env = env.instance_variable_get('@environment') + end + + context = env.context_class.new(env, "#{@target}.scss", "app/assets/stylesheets/#{@target}.scss") + + ::Sass::Engine.new(@scss, { + syntax: :scss, + cache: false, + read_cache: false, + style: Rails.env.production? ? :compressed : :expanded, + filesystem_importer: opts[:safe] ? DiscourseSafeSassImporter : DiscourseSassImporter, + sprockets: { + context: context, + environment: context.environment + } + }).render + end + +end diff --git a/lib/discourse_sass_importer.rb b/lib/sass/discourse_sass_importer.rb similarity index 73% rename from lib/discourse_sass_importer.rb rename to lib/sass/discourse_sass_importer.rb index 508520f95..05e03230b 100644 --- a/lib/discourse_sass_importer.rb +++ b/lib/sass/discourse_sass_importer.rb @@ -31,7 +31,8 @@ class DiscourseSassImporter < Sass::Importers::Filesystem "plugins" => DiscoursePluginRegistry.stylesheets, "plugins_mobile" => DiscoursePluginRegistry.mobile_stylesheets, "plugins_desktop" => DiscoursePluginRegistry.desktop_stylesheets, - "plugins_variables" => DiscoursePluginRegistry.sass_variables + "plugins_variables" => DiscoursePluginRegistry.sass_variables, + "theme_variables" => [ColorScheme::BASE_COLORS_FILE] } end @@ -45,21 +46,41 @@ class DiscourseSassImporter < Sass::Importers::Filesystem def find(name, options) if special_imports.has_key? name - stylesheets = special_imports[name] - contents = "" - stylesheets.each do |css_file| - if css_file =~ /\.scss$/ - contents << "@import '#{css_file}';" + if name == "theme_variables" + contents = "" + if color_scheme = ColorScheme.enabled + ColorScheme.base_colors.each do |name, base_hex| + override = color_scheme.colors_by_name[name] + contents << "$#{name}: ##{override ? override.hex : base_hex} !default;\n" + end else - contents << File.read(css_file) + special_imports[name].each do |css_file| + contents << File.read(css_file) + end end - depend_on(css_file) + + Sass::Engine.new(contents, options.merge( + filename: "#{name}.scss", + importer: self, + syntax: :scss + )) + else + stylesheets = special_imports[name] + contents = "" + stylesheets.each do |css_file| + if css_file =~ /\.scss$/ + contents << "@import '#{css_file}';" + else + contents << File.read(css_file) + end + depend_on(css_file) + end + Sass::Engine.new(contents, options.merge( + filename: "#{name}.scss", + importer: self, + syntax: :scss + )) end - Sass::Engine.new(contents, options.merge( - filename: "#{name}.scss", - importer: self, - syntax: :scss - )) elsif name =~ GLOB nil # globs must be relative else diff --git a/lib/sass/discourse_stylesheets.rb b/lib/sass/discourse_stylesheets.rb new file mode 100644 index 000000000..ea8f75afc --- /dev/null +++ b/lib/sass/discourse_stylesheets.rb @@ -0,0 +1,95 @@ +require_dependency 'sass/discourse_sass_compiler' + +class DiscourseStylesheets + + CACHE_PATH = 'uploads/stylesheet-cache' + + @lock = Mutex.new + + def self.stylesheet_link_tag(target = :desktop) + builder = self.new(target) + @lock.synchronize do + builder.compile unless File.exists?(builder.stylesheet_fullpath) + builder.ensure_digestless_file + %[].html_safe + end + end + + def self.compile(target = :desktop) + @lock.synchronize do + builder = self.new(target) + builder.compile + builder.stylesheet_filename + end + end + + + def initialize(target = :desktop) + @target = target + end + + def compile + scss = File.read("#{Rails.root}/app/assets/stylesheets/#{@target}.scss") + css = begin + DiscourseSassCompiler.compile(scss, @target) + rescue Sass::SyntaxError => e + Rails.logger.error "Stylesheet failed to compile for '#{@target}'! Recompiling without plugins and theming." + Rails.logger.error e.sass_backtrace_str("#{@target} stylesheet") + DiscourseSassCompiler.compile(scss + DiscourseSassCompiler.error_as_css(e, "#{@target} stylesheet"), @target, safe: true) + end + FileUtils.mkdir_p(cache_fullpath) + File.open(stylesheet_fullpath, "w") do |f| + f.puts css + end + css + end + + def ensure_digestless_file + unless File.exist?(stylesheet_fullpath_no_digest) && File.mtime(stylesheet_fullpath) == File.mtime(stylesheet_fullpath_no_digest) + FileUtils.cp(stylesheet_fullpath, stylesheet_fullpath_no_digest) + end + end + + def cache_fullpath + "#{Rails.root}/public/#{CACHE_PATH}" + end + + def stylesheet_fullpath + "#{cache_fullpath}/#{stylesheet_filename}" + end + def stylesheet_fullpath_no_digest + "#{cache_fullpath}/#{stylesheet_filename_no_digest}" + end + + def stylesheet_relpath + "/#{CACHE_PATH}/#{stylesheet_filename}" + end + def stylesheet_relpath_no_digest + "/#{CACHE_PATH}/#{stylesheet_filename_no_digest}" + end + + def stylesheet_filename + "#{@target}_#{digest}.css" + end + def stylesheet_filename_no_digest + "#{@target}.css" + end + + def digest + @digest ||= begin + # Watch for file changes unless in production env. + # In production, file changes only happen during deploy, followed by assets:precompile. + last_file_updated = if Rails.env.production? + 0 + else + [ Dir.glob("#{Rails.root}/app/assets/stylesheets/**/*.*css").map {|x| File.mtime(x) }.max, + Dir.glob("#{Rails.root}/plugins/**/assets/stylesheets/**/*.*css").map {|x| File.mtime(x) }.max ].max.to_i + end + + theme = (cs = ColorScheme.enabled) ? "#{cs.id}-#{cs.version}" : 0 + + # digest encodes the things that trigger a recompile + Digest::SHA1.hexdigest("#{RailsMultisite::ConnectionManagement.current_db}-#{theme}-#{last_file_updated}") + end + end +end diff --git a/lib/tasks/assets.rake b/lib/tasks/assets.rake index 3642621fb..15363b256 100644 --- a/lib/tasks/assets.rake +++ b/lib/tasks/assets.rake @@ -44,4 +44,16 @@ task 'assets:precompile:before' do end -task 'assets:precompile' => 'assets:precompile:before' +task 'assets:precompile:css' => 'environment' do + RailsMultisite::ConnectionManagement.each_connection do |db| + puts "Compiling css for #{db}" + [:desktop, :mobile].each do |target| + puts DiscourseStylesheets.compile(target) + end + end +end + +task 'assets:precompile' => 'assets:precompile:before' do + # Run after assets:precompile + Rake::Task["assets:precompile:css"].invoke +end diff --git a/spec/components/discourse_sass_compiler_spec.rb b/spec/components/discourse_sass_compiler_spec.rb new file mode 100644 index 000000000..8f92eaf23 --- /dev/null +++ b/spec/components/discourse_sass_compiler_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' +require_dependency 'sass/discourse_sass_compiler' + +describe DiscourseSassCompiler do + + let(:test_scss) { "body { p {color: blue;} }\n@import 'common/foundation/variables';\n@import 'plugins';" } + + describe '#compile' do + it "compiles scss" do + DiscoursePluginRegistry.stubs(:stylesheets).returns(["#{Rails.root}/spec/fixtures/scss/my_plugin.scss"]) + css = described_class.compile(test_scss, "test") + css.should include("color") + css.should include('my-plugin-thing') + end + + it "raises error for invalid scss" do + expect { + described_class.compile("this isn't valid scss", "test") + }.to raise_error(Sass::SyntaxError) + end + + it "doesn't load theme or plugins in safe mode" do + ColorScheme.expects(:enabled).never + DiscoursePluginRegistry.stubs(:stylesheets).returns(["#{Rails.root}/spec/fixtures/scss/my_plugin.scss"]) + css = described_class.compile(test_scss, "test", safe: true) + css.should_not include('my-plugin-thing') + end + end + +end diff --git a/spec/components/discourse_stylesheets_spec.rb b/spec/components/discourse_stylesheets_spec.rb new file mode 100644 index 000000000..2889716e8 --- /dev/null +++ b/spec/components/discourse_stylesheets_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' +require_dependency 'sass/discourse_stylesheets' + +describe DiscourseStylesheets do + + describe "compile" do + it "can compile desktop bundle" do + DiscoursePluginRegistry.stubs(:stylesheets).returns(["#{Rails.root}/spec/fixtures/scss/my_plugin.scss"]) + builder = described_class.new(:desktop) + builder.compile.should include('my-plugin-thing') + FileUtils.rm builder.stylesheet_fullpath + end + + it "can compile mobile bundle" do + DiscoursePluginRegistry.stubs(:mobile_stylesheets).returns(["#{Rails.root}/spec/fixtures/scss/my_plugin.scss"]) + builder = described_class.new(:mobile) + builder.compile.should include('my-plugin-thing') + FileUtils.rm builder.stylesheet_fullpath + end + + it "can fallback when css is bad" do + DiscoursePluginRegistry.stubs(:stylesheets).returns([ + "#{Rails.root}/spec/fixtures/scss/my_plugin.scss", + "#{Rails.root}/spec/fixtures/scss/broken.scss" + ]) + builder = described_class.new(:desktop) + builder.compile.should_not include('my-plugin-thing') + FileUtils.rm builder.stylesheet_fullpath + end + end + +end diff --git a/spec/controllers/admin/color_schemes_controller_spec.rb b/spec/controllers/admin/color_schemes_controller_spec.rb index 70f2b42e4..c4c24db48 100644 --- a/spec/controllers/admin/color_schemes_controller_spec.rb +++ b/spec/controllers/admin/color_schemes_controller_spec.rb @@ -11,8 +11,8 @@ describe Admin::ColorSchemesController do name: 'Such Design', enabled: true, colors: [ - {name: '$primary_background_color', hex: 'FFBB00', opacity: '100'}, - {name: '$secondary_background_color', hex: '888888', opacity: '70'} + {name: 'primary', hex: 'FFBB00'}, + {name: 'secondary', hex: '888888'} ] } } } @@ -40,6 +40,14 @@ describe Admin::ColorSchemesController do xhr :post, :create, valid_params ::JSON.parse(response.body)['id'].should be_present end + + it "returns failure with invalid params" do + params = valid_params + params[:color_scheme][:colors][0][:hex] = 'cool color please' + xhr :post, :create, valid_params + response.should_not be_success + ::JSON.parse(response.body)['errors'].should be_present + end end describe "update" do @@ -56,6 +64,16 @@ describe Admin::ColorSchemesController do xhr :put, :update, valid_params.merge(id: existing.id) ::JSON.parse(response.body)['id'].should be_present end + + it "returns failure with invalid params" do + color_scheme = Fabricate(:color_scheme) + params = valid_params.merge(id: color_scheme.id) + params[:color_scheme][:colors][0][:name] = color_scheme.colors.first.name + params[:color_scheme][:colors][0][:hex] = 'cool color please' + xhr :put, :update, params + response.should_not be_success + ::JSON.parse(response.body)['errors'].should be_present + end end describe "destroy" do diff --git a/spec/fabricators/color_scheme_color_fabricator.rb b/spec/fabricators/color_scheme_color_fabricator.rb index 2bfe62b93..6cac52df7 100644 --- a/spec/fabricators/color_scheme_color_fabricator.rb +++ b/spec/fabricators/color_scheme_color_fabricator.rb @@ -1,6 +1,5 @@ Fabricator(:color_scheme_color) do color_scheme - name { sequence(:name) {|i| "$color_#{i}" } } + name { sequence(:name) {|i| "color_#{i}" } } hex "333333" - opacity 100 end diff --git a/spec/fixtures/scss/broken.scss b/spec/fixtures/scss/broken.scss new file mode 100644 index 000000000..7c9db0d74 --- /dev/null +++ b/spec/fixtures/scss/broken.scss @@ -0,0 +1 @@ +this isn't valid scss, so it will fail to compile diff --git a/spec/fixtures/scss/my_plugin.scss b/spec/fixtures/scss/my_plugin.scss new file mode 100644 index 000000000..176ac00d9 --- /dev/null +++ b/spec/fixtures/scss/my_plugin.scss @@ -0,0 +1,3 @@ +.my-plugin-thing { + color: blue +} diff --git a/spec/models/color_scheme_color_spec.rb b/spec/models/color_scheme_color_spec.rb new file mode 100644 index 000000000..e183e5914 --- /dev/null +++ b/spec/models/color_scheme_color_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe ColorSchemeColor do + def test_invalid_hex(hex) + c = described_class.new(hex: hex) + c.should_not be_valid + c.errors[:hex].should be_present + end + + it "validates hex value" do + ['fff', 'ffffff', '333333', '333', '0BeeF0'].each do |hex| + described_class.new(hex: hex).should be_valid + end + ['fffff', 'ffff', 'ff', 'f', '00000', '00', 'cheese', '#666666', '#666', '555 666'].each do |hex| + test_invalid_hex(hex) + end + end +end diff --git a/spec/models/color_scheme_spec.rb b/spec/models/color_scheme_spec.rb index 88e59324e..a1e32f450 100644 --- a/spec/models/color_scheme_spec.rb +++ b/spec/models/color_scheme_spec.rb @@ -2,10 +2,28 @@ require 'spec_helper' describe ColorScheme do + describe '#base_colors' do + it 'parses the colors.scss file and returns a hash' do + File.stubs(:readlines).with(described_class::BASE_COLORS_FILE).returns([ + '$primary: #333333 !default;', + '$secondary: #ffffff !default; ', + '$highlight: #ffff4d;', + ' $danger:#e45735 !default;', + ]) + + colors = described_class.base_colors + colors.should be_a(Hash) + colors['primary'].should == '333333' + colors['secondary'].should == 'ffffff' + colors['highlight'].should == 'ffff4d' + colors['danger'].should == 'e45735' + end + end + let(:valid_params) { {name: "Best Colors Evar", enabled: true, colors: valid_colors} } let(:valid_colors) { [ - {name: '$primary_background_color', hex: 'FFBB00', opacity: '100'}, - {name: '$secondary_background_color', hex: '888888', opacity: '70'} + {name: '$primary_background_color', hex: 'FFBB00'}, + {name: '$secondary_background_color', hex: '888888'} ]} describe "new" do @@ -31,8 +49,8 @@ describe ColorScheme do end describe "#enabled" do - it "returns the base color scheme when there is no enabled record" do - described_class.enabled.id.should == 1 + it "returns nil when there is no enabled record" do + described_class.enabled.should be_nil end it "returns the enabled color scheme" do diff --git a/spec/models/site_customization_spec.rb b/spec/models/site_customization_spec.rb index 3bfb89d37..702e3c1c3 100644 --- a/spec/models/site_customization_spec.rb +++ b/spec/models/site_customization_spec.rb @@ -155,12 +155,12 @@ describe SiteCustomization do it 'should compile scss' do c = SiteCustomization.create!(user_id: user.id, name: "test", stylesheet: '$black: #000; #a { color: $black; }', header: '') - c.stylesheet_baked.should == "#a{color:#000}\n" + ["#a{color:#000;}", "#a{color:black;}"].should include(c.stylesheet_baked.gsub(' ', '').gsub("\n", '')) end it 'should compile mobile scss' do c = SiteCustomization.create!(user_id: user.id, name: "test", stylesheet: '', header: '', mobile_stylesheet: '$black: #000; #a { color: $black; }', mobile_header: '') - c.mobile_stylesheet_baked.should == "#a{color:#000}\n" + ["#a{color:#000;}", "#a{color:black;}"].should include(c.mobile_stylesheet_baked.gsub(' ', '').gsub("\n", '')) end it 'should allow including discourse styles' do diff --git a/spec/services/color_scheme_revisor_spec.rb b/spec/services/color_scheme_revisor_spec.rb index 15ab86d7a..374a48ad5 100644 --- a/spec/services/color_scheme_revisor_spec.rb +++ b/spec/services/color_scheme_revisor_spec.rb @@ -25,14 +25,23 @@ describe ColorSchemeRevisor do color_scheme.reload.should_not be_enabled end - it "can change colors" do - described_class.revise(color_scheme, valid_params.merge(colors: [ - {name: color.name, hex: 'BEEF99', opacity: 99} + def test_color_change(color_scheme_arg, expected_enabled) + described_class.revise(color_scheme_arg, valid_params.merge(colors: [ + {name: color.name, hex: 'BEEF99'} ])) - color_scheme.reload - color_scheme.colors.size.should == 1 - color_scheme.colors.first.hex.should == 'BEEF99' - color_scheme.colors.first.opacity.should == 99 + color_scheme_arg.reload + color_scheme_arg.enabled.should == expected_enabled + color_scheme_arg.colors.size.should == 1 + color_scheme_arg.colors.first.hex.should == 'BEEF99' + end + + it "can change colors of a color scheme that's not enabled" do + test_color_change(color_scheme, false) + end + + it "can change colors of the enabled color scheme" do + color_scheme.update_attribute(:enabled, true) + test_color_change(color_scheme, true) end it "disables other color scheme before enabling" do @@ -42,6 +51,17 @@ describe ColorSchemeRevisor do color_scheme.reload.enabled.should == true end + it "doesn't make changes when a color is invalid" do + expect { + cs = described_class.revise(color_scheme, valid_params.merge(colors: [ + {name: color.name, hex: 'OOPS'} + ])) + cs.should_not be_valid + cs.errors.should be_present + }.to_not change { color_scheme.reload.version } + color_scheme.colors.first.hex.should == color.hex + end + describe "versions" do it "doesn't create a new version if colors is not given" do expect { @@ -50,25 +70,23 @@ describe ColorSchemeRevisor do end it "creates a new version if colors have changed" do - old_hex, old_opacity = color.hex, color.opacity + old_hex = color.hex expect { described_class.revise(color_scheme, valid_params.merge(colors: [ - {name: color.name, hex: 'BEEF99', opacity: 99} + {name: color.name, hex: 'BEEF99'} ])) }.to change { color_scheme.reload.version }.by(1) old_version = ColorScheme.find_by(versioned_id: color_scheme.id, version: (color_scheme.version - 1)) old_version.should_not be_nil old_version.colors.count.should == color_scheme.colors.count old_version.colors_by_name[color.name].hex.should == old_hex - old_version.colors_by_name[color.name].opacity.should == old_opacity color_scheme.colors_by_name[color.name].hex.should == 'BEEF99' - color_scheme.colors_by_name[color.name].opacity.should == 99 end it "doesn't create a new version if colors have not changed" do expect { described_class.revise(color_scheme, valid_params.merge(colors: [ - {name: color.name, hex: color.hex, opacity: color.opacity} + {name: color.name, hex: color.hex} ])) }.to_not change { color_scheme.reload.version } end @@ -85,10 +103,10 @@ describe ColorSchemeRevisor do end context 'when there are previous versions' do - let(:new_color_params) { {name: color.name, hex: 'BEEF99', opacity: 99} } + let(:new_color_params) { {name: color.name, hex: 'BEEF99'} } before do - @prev_hex, @prev_opacity = color.hex, color.opacity + @prev_hex = color.hex described_class.revise(color_scheme, valid_params.merge(colors: [ new_color_params ])) end @@ -99,9 +117,7 @@ describe ColorSchemeRevisor do }.to change { color_scheme.reload.version }.by(-1) color_scheme.colors.size.should == 1 color_scheme.colors.first.hex.should == @prev_hex - color_scheme.colors.first.opacity.should == @prev_opacity color_scheme.colors_by_name[new_color_params[:name]].hex.should == @prev_hex - color_scheme.colors_by_name[new_color_params[:name]].opacity.should == @prev_opacity end it "destroys the old version's record" do