Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for multiple themes #4959

Merged
merged 11 commits into from Sep 19, 2017
8 changes: 8 additions & 0 deletions app/controllers/application_controller.rb
Expand Up @@ -12,6 +12,7 @@ class ApplicationController < ActionController::Base

helper_method :current_account
helper_method :current_session
helper_method :current_theme
helper_method :single_user_mode?

rescue_from ActionController::RoutingError, with: :not_found
Expand Down Expand Up @@ -77,6 +78,13 @@ def current_session
@current_session ||= SessionActivation.find_by(session_id: cookies.signed['_session_id'])
end

def current_theme
if current_account and Themes.instance.names.include? current_account.user.setting_theme
return current_account&.user&.setting_theme
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it needs &. operator here. If they gets nil, it is a programming error and should fail rather than returning nil.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with aki, also there is current_account.user.setting_theme without &. in the previous line.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should just be current_user, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should cover most cases and make more sense?

return Setting.default_settings['theme'] unless Themes.instance.names.include? current_user&.setting_theme
return current_user.setting_theme

end
Setting.default_settings['theme']
end

def cache_collection(raw, klass)
return raw unless klass.respond_to?(:with_includes)

Expand Down
1 change: 1 addition & 0 deletions app/controllers/settings/preferences_controller.rb
Expand Up @@ -41,6 +41,7 @@ def user_settings_params
:setting_auto_play_gif,
:setting_system_font_ui,
:setting_noindex,
:setting_theme,
notification_emails: %i(follow follow_request reblog favourite mention digest),
interactions: %i(must_be_follower must_be_following)
)
Expand Down
3 changes: 0 additions & 3 deletions app/javascript/packs/common.js
@@ -1,9 +1,6 @@
import { start } from 'rails-ujs';

// import default stylesheet with variables
require('font-awesome/css/font-awesome.css');
require('mastodon-application-style');

require.context('../images/', true);

start();
16 changes: 16 additions & 0 deletions app/lib/themes.rb
@@ -0,0 +1,16 @@
# frozen_string_literal: true

require 'singleton'
require 'yaml'

class Themes
include Singleton

def initialize
@conf = YAML.load_file(Rails.root.join('config', 'themes.yml'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't remember if this needs to be frozen or not in modern rails

end

def names
@conf.keys
end
end
5 changes: 5 additions & 0 deletions app/lib/user_settings_decorator.rb
Expand Up @@ -25,6 +25,7 @@ def process_update
user.settings['auto_play_gif'] = auto_play_gif_preference
user.settings['system_font_ui'] = system_font_ui_preference
user.settings['noindex'] = noindex_preference
user.settings['theme'] = theme_preference
end

def merged_notification_emails
Expand Down Expand Up @@ -67,6 +68,10 @@ def noindex_preference
boolean_cast_setting 'setting_noindex'
end

def theme_preference
settings['setting_theme']
end

def boolean_cast_setting(key)
settings[key] == '1'
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/user.rb
Expand Up @@ -110,6 +110,10 @@ def setting_noindex
settings.noindex
end

def setting_theme
settings.theme
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternatively could do fallback here but that seems less good.

end

def token_for_app(a)
return nil if a.nil? || a.owner != self
Doorkeeper::AccessToken
Expand Down
1 change: 1 addition & 0 deletions app/views/layouts/application.html.haml
Expand Up @@ -19,6 +19,7 @@
= title

= stylesheet_pack_tag 'common', media: 'all'
= stylesheet_pack_tag current_theme, media: 'all'
= javascript_pack_tag 'common', integrity: true, crossorigin: 'anonymous'

%link{ href: asset_pack_path('features/getting_started.js'), crossorigin: 'anonymous', rel: 'preload', as: 'script' }/
Expand Down
1 change: 1 addition & 0 deletions app/views/layouts/embedded.html.haml
Expand Up @@ -5,6 +5,7 @@
%meta{ name: 'robots', content: 'noindex' }/

= stylesheet_pack_tag 'common', media: 'all'
= stylesheet_pack_tag current_theme, media: 'all'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases, embeds will be seen by non-logged in users. So I think it should be fixed to default theme, and it shouldn't be changed by user preferences.

= javascript_pack_tag 'common', integrity: true, crossorigin: 'anonymous'
= javascript_pack_tag "locale_#{I18n.locale}", integrity: true, crossorigin: 'anonymous'
= javascript_pack_tag 'public', integrity: true, crossorigin: 'anonymous'
Expand Down
2 changes: 2 additions & 0 deletions app/views/settings/preferences/show.html.haml
Expand Up @@ -5,6 +5,8 @@
= render 'shared/error_messages', object: current_user

.fields-group
= f.input :setting_theme, collection: Themes.instance.names, label_method: lambda { |theme| safe_join([I18n.t("themes.#{theme}", default: theme)])}, wrapper: :with_label, include_blank: false

= f.input :locale,
collection: I18n.available_locales,
wrapper: :with_label,
Expand Down
2 changes: 2 additions & 0 deletions config/locales/en.yml
Expand Up @@ -449,6 +449,8 @@ en:
settings: Settings
two_factor_authentication: Two-factor Authentication
your_apps: Your applications
themes:
default: Mastodon
statuses:
open_in_web: Open in web
over_character_limit: character limit of %{max} exceeded
Expand Down
2 changes: 2 additions & 0 deletions config/locales/simple_form.en.yml
Expand Up @@ -13,6 +13,7 @@ en:
one: <span class="note-counter">1</span> character left
other: <span class="note-counter">%{count}</span> characters left
setting_noindex: Affects your public profile and status pages
setting_theme: Affects how Mastodon looks when you're logged in from any device.
imports:
data: CSV file exported from another Mastodon instance
sessions:
Expand Down Expand Up @@ -44,6 +45,7 @@ en:
setting_noindex: Opt-out of search engine indexing
setting_system_font_ui: Use system's default font
setting_unfollow_modal: Show confirmation dialog before unfollowing someone
setting_theme: Site theme
severity: Severity
type: Import type
username: Username
Expand Down
1 change: 1 addition & 0 deletions config/settings.yml
Expand Up @@ -24,6 +24,7 @@ defaults: &defaults
auto_play_gif: false
system_font_ui: false
noindex: false
theme: 'default'
notification_emails:
follow: false
reblog: false
Expand Down
1 change: 1 addition & 0 deletions config/themes.yml
@@ -0,0 +1 @@
default: styles/application.scss
4 changes: 4 additions & 0 deletions config/webpack/configuration.js
Expand Up @@ -9,6 +9,9 @@ const configPath = resolve('config', 'webpacker.yml');
const loadersDir = join(__dirname, 'loaders');
const settings = safeLoad(readFileSync(configPath), 'utf8')[env.NODE_ENV];

const themePath = resolve('config', 'themes.yml');
const themes = safeLoad(readFileSync(themePath), 'utf8');

function removeOuterSlashes(string) {
return string.replace(/^\/*/, '').replace(/\/*$/, '');
}
Expand All @@ -29,6 +32,7 @@ const output = {

module.exports = {
settings,
themes,
env,
loadersDir,
output,
Expand Down
36 changes: 18 additions & 18 deletions config/webpack/shared.js
@@ -1,34 +1,38 @@
// Note: You must restart bin/webpack-dev-server for changes to take effect

const { existsSync } = require('fs');
const webpack = require('webpack');
const { basename, dirname, join, relative, resolve, sep } = require('path');
const { sync } = require('glob');
const ExtractTextPlugin = require('extract-text-webpack-plugin');
const ManifestPlugin = require('webpack-manifest-plugin');
const extname = require('path-complete-extname');
const { env, settings, output, loadersDir } = require('./configuration.js');
const { env, settings, themes, output, loadersDir } = require('./configuration.js');
const localePackPaths = require('./generateLocalePacks');

const extensionGlob = `**/*{${settings.extensions.join(',')}}*`;
const entryPath = join(settings.source_path, settings.source_entry_path);
const packPaths = sync(join(entryPath, extensionGlob));
const entryPacks = [...packPaths, ...localePackPaths].filter(path => path !== join(entryPath, 'custom.js'));

const customApplicationStyle = resolve(join(settings.source_path, 'styles/custom.scss'));
const originalApplicationStyle = resolve(join(settings.source_path, 'styles/application.scss'));
const themePaths = Object.keys(themes).reduce(
(themePaths, name) => {
themePaths[name] = resolve(join(settings.source_path, themes[name]));
return themePaths;
}, {});

module.exports = {
entry: entryPacks.reduce(
(map, entry) => {
const localMap = map;
let namespace = relative(join(entryPath), dirname(entry));
if (namespace === join('..', '..', '..', 'tmp', 'packs')) {
namespace = ''; // generated by generateLocalePacks.js
}
localMap[join(namespace, basename(entry, extname(entry)))] = resolve(entry);
return localMap;
}, {}
entry: Object.assign(
entryPacks.reduce(
(map, entry) => {
const localMap = map;
let namespace = relative(join(entryPath), dirname(entry));
if (namespace === join('..', '..', '..', 'tmp', 'packs')) {
namespace = ''; // generated by generateLocalePacks.js
}
localMap[join(namespace, basename(entry, extname(entry)))] = resolve(entry);
return localMap;
}, {}
), themePaths
),

output: {
Expand Down Expand Up @@ -67,10 +71,6 @@ module.exports = {
],

resolve: {
alias: {
'mastodon-application-style': existsSync(customApplicationStyle) ?
customApplicationStyle : originalApplicationStyle,
},
extensions: settings.extensions,
modules: [
resolve(settings.source_path),
Expand Down