Native theme feature feedback, bugs, thoughts, feelings, prayers

[cpradio]

But it is at the very top… did you add it in another place too?

[jomaxro (Joshua Rosenfeld)]

Yes, I added it to all 3 fields - and it only appeared in Header. I wanted to ensure the HTML was valid and would appear somewhere…

[sam (Sam Saffron)]

This is fixed per:

https://github.com/discourse/discourse/commit/def7348777832f454c92f07c75ced769b5ae1718

Also, made it a bit easier to preview themes and fixed a bug where when you deleted a theme users that had it selected would get an error.

IMPORTANT I changed the cookie name from “preview_style” to “theme_key” which is more correct, this means if you had a user selected theme on meta you will need to select it again.

[sam (Sam Saffron)]

@jomaxro

Strange CSS bug for line numbers when there are >9 lines (double digits) displayed in the age editor.

ouch … adding margin to this editor has been … tricky.

[david (David Taylor)]

Got a lot of users complaining of persistant error 500s after changing theme. Only way to remedy seems to be for them to reset their cookies… I’m getting hundreds of occurences of this in the log:

NoMethodError (undefined method `theme_user_keys' for #<Class:0x007f955844e388>)
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-4.2.8/lib/active_record/dynamic_matchers.rb:26:in `method_missing'

backtrace

/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-4.2.8/lib/active_record/dynamic_matchers.rb:26:in `method_missing'
/var/www/discourse/lib/guardian.rb:304:in `allow_theme?'
/var/www/discourse/app/controllers/application_controller.rb:269:in `handle_theme'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:432:in `block in make_lambda'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:164:in `block in halting'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:504:in `block in call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:504:in `each'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:504:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:92:in `__run_callbacks__'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:778:in `_run_process_action_callbacks'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:81:in `run_callbacks'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/abstract_controller/callbacks.rb:19:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_controller/metal/rescue.rb:29:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_controller/metal/instrumentation.rb:32:in `block in process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications.rb:164:in `block in instrument'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/instrumenter.rb:20:in `instrument'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications.rb:164:in `instrument'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_controller/metal/instrumentation.rb:30:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_controller/metal/params_wrapper.rb:250:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-4.2.8/lib/active_record/railties/controller_runtime.rb:18:in `process_action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/abstract_controller/base.rb:137:in `process'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionview-4.2.8/lib/action_view/rendering.rb:30:in `process'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-mini-profiler-0.10.1/lib/mini_profiler/profiling_methods.rb:76:in `block in profile_method'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_controller/metal.rb:196:in `dispatch'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_controller/metal/rack_delegation.rb:13:in `dispatch'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_controller/metal.rb:237:in `block in action'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/routing/route_set.rb:74:in `dispatch'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/routing/route_set.rb:43:in `serve'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/routing/mapper.rb:49:in `serve'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/journey/router.rb:43:in `block in serve'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/journey/router.rb:30:in `each'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/journey/router.rb:30:in `serve'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/routing/route_set.rb:817:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-protection-1.5.3/lib/rack/protection/frame_options.rb:31:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:189:in `call!'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/strategy.rb:167:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/omniauth-1.6.1/lib/omniauth/builder.rb:63:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/conditionalget.rb:25:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/head.rb:13:in `call'
/var/www/discourse/lib/middleware/anonymous_cache.rb:138:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/middleware/params_parser.rb:27:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/middleware/flash.rb:260:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/session/abstract/id.rb:225:in `context'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/session/abstract/id.rb:220:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/middleware/cookies.rb:560:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-4.2.8/lib/active_record/query_cache.rb:36:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/connection_pool.rb:653:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:88:in `__run_callbacks__'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:778:in `_run_call_callbacks'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/activesupport-4.2.8/lib/active_support/callbacks.rb:81:in `run_callbacks'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/middleware/callbacks.rb:27:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/middleware/remote_ip.rb:78:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/middleware/debug_exceptions.rb:17:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/middleware/show_exceptions.rb:30:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/logster-1.2.7/lib/logster/middleware/reporter.rb:31:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-4.2.8/lib/rails/rack/logger.rb:38:in `call_app'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-4.2.8/lib/rails/rack/logger.rb:22:in `call'
/var/www/discourse/config/initializers/100-quiet_logger.rb:17:in `call_with_quiet_assets'
/var/www/discourse/config/initializers/100-silence_logger.rb:29:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/actionpack-4.2.8/lib/action_dispatch/middleware/request_id.rb:21:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/methodoverride.rb:22:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/runtime.rb:18:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/sendfile.rb:113:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-mini-profiler-0.10.1/lib/mini_profiler/profiler.rb:171:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/message_bus-2.0.2/lib/message_bus/rack/middleware.rb:62:in `call'
/var/www/discourse/lib/middleware/request_tracker.rb:73:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-4.2.8/lib/rails/engine.rb:518:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-4.2.8/lib/rails/application.rb:165:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-4.2.8/lib/rails/railtie.rb:194:in `public_send'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/railties-4.2.8/lib/rails/railtie.rb:194:in `method_missing'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/urlmap.rb:66:in `block in call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/urlmap.rb:50:in `each'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/rack-1.6.5/lib/rack/urlmap.rb:50:in `call'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.2.0/lib/unicorn/http_server.rb:562:in `process_client'
/var/www/discourse/lib/scheduler/defer.rb:85:in `process_client'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.2.0/lib/unicorn/http_server.rb:658:in `worker_loop'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.2.0/lib/unicorn/http_server.rb:508:in `spawn_missing_workers'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.2.0/lib/unicorn/http_server.rb:132:in `start'
/var/www/discourse/vendor/bundle/ruby/2.3.0/gems/unicorn-5.2.0/bin/unicorn:126:in `<top (required)>'
/var/www/discourse/vendor/bundle/ruby/2.3.0/bin/unicorn:22:in `load'
/var/www/discourse/vendor/bundle/ruby/2.3.0/bin/unicorn:22:in `<main>'

Running absolute latest version of tests-passed

Edit: I can reproduce this on Meta (if you want to keep accessing the forum, don’t do these steps! )

1. Be on default theme
2. Go to user preferences
3. Change to dark theme
4. Click logo to go to homepage
5. Error 500

[david (David Taylor)]

Looks like a typo to me: user_theme_keys vs. theme_user_keys.

Ironically, bug only applies to non-staff - probably why it didn’t get noticed

https://github.com/discourse/discourse/pull/4813

[codinghorror (Jeff Atwood)]

Thank you for this fix!

[david (David Taylor)]

@tophee

I don’t like that fully fledged themes and theme-elements are presented in one list of “themes”, as if they are the same (of course, you can argue that technically they are the same, it’s all a matter of perspective, but for the admin user who’d never treat the design element “Avatar shadow” as a theme in it’s own right, it is not the ideal way of presenting things. I wonder if I’m asking for too much when I suggest to keep themes and theme elements seperate, i.e. in a seperate Theme Elements tab?

I agree that it can be quite confusing having full ‘themes’ and what you referred to as ‘theme elements’ in the same list. While they are technically the same, they serve very different purposes. On my forum, I’ve prefixed ‘theme elements’ with a ‘z’ so that when sorted alphabetically they are separated. This clearly isn’t ideal, and I wonder whether splitting them into two lists would be beneficial. Here’s a quick mockup:

Themes would automagically move between the two lists when you toggle the “Theme can be selected by users” checkbox.

---

@tophee

bring back the asterisk (or some other visual marker) for theme elements that are in use in another theme

How about having a read-only “This theme is used by” section in addition to the “Included themes” section?

[codinghorror (Jeff Atwood)]

Any other pending bits here to tend to @sam?

[sam (Sam Saffron)]

Yeah a few items in the OP to take care of

[david (David Taylor)]

Just noticed that when a new theme is created, the placeholder for the name is “New Style”. Might it be better to change that to “New Theme” to avoid any potential confusion? (ultimate bikeshedding I know)

[david (David Taylor)]

I’m getting these 3 warnings in the chrome JS console on my own forum, and here on meta. I’m guessing they’re probably related to the new theme stuff:

[sam (Sam Saffron)]

yeah I just fixed that in dev should be deployed shortly

[sam (Sam Saffron)]

I fixed a bunch today:

https://github.com/discourse/discourse/commit/bbeb69ddc9016d7ea911e15c5ddcc54202b46b29

https://github.com/discourse/discourse/commit/8ffb918824644bf2d7766586566b4db52a055ce1

and I fixed the Nitpicky Mobile not positioned right

Still left to look at:

• Better errors for bad CSS or HTML

• Delete color scheme issues

[sam (Sam Saffron)]

@jomaxro

Found another. Deleting a color scheme and quickly selecting another before the UI refreshes results in the “deleted” theme appearing instead of the non-deleted one. A refresh is required to resolve this.

Fixed per:

FIX: destroying color scheme could remove wrong item from UI

[sam (Sam Saffron)]

OK… this concludes the last of the issues reported by the OP

https://github.com/discourse/discourse/commit/7eabb90b710a4ad6527c37c2765323268f8dde14

No more errors in weird places …

[sam (Sam Saffron)]

Also… @codinghorror was asking so I added some paintbrushes

[jomaxro (Joshua Rosenfeld)]

@sam thanks for all your work on this!

@sam

we can add an extra visual cue for “enabled child theme”

Never added this to the OP. Away from a PC for the day, did this get handled?

[jomaxro (Joshua Rosenfeld)]

Got another .

I cannot add a Theme Component to a newly created theme without first refreshing the page. Received the following error:

Also noticed there’s some console logging going on. Is that intentional? PR to remove it if not:
https://github.com/discourse/discourse/pull/4827

Edit: Merged
Edit 2: Another .

Just got this error when selecting a theme in user preferences. The error appeared when selecting a theme in the menu, before clicking save. Theme was applied, but I had to refresh every page to see it (including the preferences page I was on at the moment:

[jomaxro (Joshua Rosenfeld)]

Sorry for the triple post…

Is there a different way the remove theme component buttons could appear? Currently they’re all over the place (due to the different length names. Not sure if the X before the title would look good - maybe the button could replace the bullet to avoid duplication?