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

I noticed a couple of issues after updating my instance to play around with the new native themes.

  1. :white_check_mark: Included Themes (subthemes) seem to appear in random order. Should/could this be alphabetical, like the list on the left?

  2. :white_check_mark: Clicking Edit CSS/HTML does not indicate what page I’ve navigated to. I’m assuming it’s common → CSS, but the UI doesn’t make it clear.

  3. :white_check_mark: CSS editor is giving incorrect errors and bad warnings.

  4. :white_check_mark: Clicking Themes while in the CSS/HTML editor leads to a blank-ish page. Clicking again does not resolve. Clicking a different tab and returning, or refreshing the page is required.

  5. :x: Not a bug, see post 10. Migration error: Had a script in the Desktop </body> tab of a customization - it is now in the Common </body>.

  6. :white_check_mark: Clicking on a tab in the editor does not focus the cursor in the text area (Chrome Desktop 57 - Stable). A second click in the text area is needed before anything can be typed/selected with keyboard shortcuts.

  7. If there is invalid SCSS, errors are displayed on public pages!

  8. :white_check_mark: Save button doesn’t seem to work consistently…I don’t have a consistent repro yet - but while trying to move CSS from Desktop/Mobile to Common, I’ve noticed that after deleting everything from Desktop, clicking save, switching tabs, and then returning the CSS I just completely deleted is still there. It appears to be a purely UI issue. Refreshing the page after clicking save does show that the content is gone. Is the UI caching the view for some reason? OK, now I’m just confused. Selected all text, deleted it, clicked save, waited until saving... went away, refreshed page, text was still there! Something’s wonky with the save button…
    This might be it…this seems to be consistent:

  9. Have a CSS customization (for example on Desktop).

  10. Select everything and delete.

  11. Click Save.

  12. Navigate to another tab (like Mobile).

  13. Make any edit.

  14. Click Save.

  15. Return to previous tab (Desktop).
    Even after a refresh, the content deleted in step 2 is still there. However, if a refresh is done between steps 3 & 4, the content is removed successfully even after completing the remaining steps.

  16. :white_check_mark: Strange CSS bug for line numbers when there are >9 lines (double digits) displayed in the age editor. This occurs regardless of the content of the editor or browser width.

  17. :white_check_mark: 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.

  18. :white_check_mark: Sorting of themes seems to be case-sensitive after this commit.

  19. :white_check_mark: Nitpicky, but… when looking at Edit CSS/HTML for a theme, the work Mobile is lower than the words Common and Desktop. It appears to be caused by the line below. I understand why the icon size is increased (boy is it tiny by default) - but is there a better way to do this without also shifting the text down?:

https://github.com/discourse/discourse/blob/master/app/assets/stylesheets/common/admin/customize.scss#L65

11 Likes

The new native themes are really a big step forward! Congrats! It’s also good that existing customizations are being smoothly adopted into the new themes framework. Or maybe not so smoothly? At least from an admin-user perspective I’m having some difficulties getting my head around the new way of native themes. I suppose I’m not the only one so I’d like to do some thinking out loud here to try and come to grips with it all. Please confirm or correct as appropriate:

  • I guess the first thing to understand is that your “main” page for customization is no longer the CSS/HTML tab (now called “Themes”) but one step further down: the “Default” sub-tab. This is where I get an overview of my default theme’s settings, including the colour scheme. (It’s a bit confusing at first, because it is no longer evident which colour scheme is being used by looking at the Color tab.) Wouldn’t it make sense to display the default theme sub-tab by default when navigating to the themes tab?
  • Each of my previous customization elements (such as: “Avatar shadow” or “Add Link to hamburger menu”) has now become a theme in it’s own right. I understand that this is a technical necessity for transitioning but not really the way you would build a native theme from scratch, right? Or maybe it is, because it allows me to easily turn them on and off like previously (and to re-use them in various themes)? But if that is the idea, 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?
  • When you first look at the new Themes tab, it looks as if all your previous customization elements have been deactivated (because the don’t have an asterisk any more), but that is not the case. Which ones are used can now be seen on the Default-theme sub-tab. But I have some gripes with that: In particular, there is no more easy way of seeing which theme-elements are currently not used in any theme (and might therefore be deleted). So, I’d like to suggest to bring back the asterisk (or some other visual marker) for theme elements that are in use in another theme, similar to the user icon that appears if users can select a theme(-element). That way, it’d be much easier to read the themes list (and perhaps this would also make my above suggestion about a separate theme-element tab superfluous).
4 Likes

After playing with the new native themes for a bit, I wanted to share my general thoughts.

  1. Included Themes list items should be clickable, linking to theme as if clicked in left-side list.
  2. The new “custom sections” display is great! It’s so easy to see at a glance what has been customized - really helpful when multiple customizations are in use.
  3. Thank you, thank you, THANK YOU!! for the new “Common” CSS/HTML tab. No more copying/pasting and wondering if you forgot something.

Edit:

  1. Which CSS tab takes priority? If I have a property on both the Common and Desktop tab (for example) with different values, what will happen?
6 Likes

This feature is brand new and actively a work in progress so PLEASE FOR THE LOVE OF GOD keep all the feedback in one topic as @sam works through it all.

9 Likes

Sounds good. I figured separating “bugs” from general thoughts would be preferred, but one topic works too!

FYI, to my knowledge the only type of comments CSS supports are /* your comment here */

Now SCSS does support // comment styles, but maybe the validator isn’t taking into account SCSS (which seems plausible considering it didn’t like the nesting of classes)

Edited:
Actually, I see why you are getting the warnings, they are legit. It is telling you that

.list-controls .category-breadcrumb {
  a.badge-category {
    padding: 6px 8px;
  }
  a.badge-category.category-dropdown-button {
    padding: 6px 5px;
  }
}

Can be changed to

.list-controls .category-breadcrumb {
  a.badge-category {
    padding: 6px 8px;
    
    &.category-dropdown-button {
      padding: 6px 5px;
    }
  }
}
3 Likes

Well, the warnings are gone, but now I’ve got new “errors”. I’m going to hold off on trying more changes until Sam is able to chime in on any potential changes to the CSS validator during the change - as none of these errors/warnings appeared before the recent changes.

Edit: With the first commit listed below this is resolved.

fixes committed today:

https://github.com/discourse/discourse/commit/37d4dd4a4bddb201111e541888a1f61aa6789f89

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

https://github.com/discourse/discourse/commit/679b6548e66af5aec94e23ae6bf83ac4c74aed7d

9 Likes

That is not a bug:

It was in “common” previously, the UI was just super misleading. It said on the screen “desktop” but really it was common. Mobile did not have /head or /body

2 Likes

Hmm I am not sure what to do here, maybe I will stop using the CSS error magic in themes and instead add a GIANT red box in show theme page that tells you your scss is all jacked up and unusable.

3 Likes

Makes sense. Updated OP.

I’d honestly rather have a GIANT red box then an error visible to non-admins. What is “CSS error magic”?

this :slight_smile:

https://github.com/discourse/discourse/blob/master/lib/stylesheet/manager.rb#L121-L122

But I can do much better for themes, just leave the .css blank and display an error on the edit theme page.

3 Likes

Not following it is still there:

2 Likes

The concern that @tophee has (and @pfaffman had in a duplicate topic) is that there’s no indication which themes are in use as “Included Themes” elsewhere. For example, in my case I have 16 themes listed, but only 1 asterisk. At a glance, I have no way of knowing that 14 of the 15 themes without an asterisk are being used, as they’re included in the enabled theme. Does that make sense?

5 Likes

yeah I follow, we can add an extra visual cue for “enabled child theme” and improve the UI to give better visibility of parents.

this is now fixed btw

https://github.com/discourse/discourse/commit/43e4fc03ef60ea8731f00ecd43e45894bca4d234

7 Likes

A bit awkward to fix but here is the focus fix

https://github.com/discourse/discourse/commit/9927489f4e318ad197b75f377a5642d22ed2fb54

5 Likes

One last fix for the day … the rest will have to wait for another day.

https://github.com/discourse/discourse/commit/809fbb25ce39c72aaff143f805035b3008f20e75

6 Likes

Love the new common css, we’ll gain a lot of time. With a heavily customized forum.

I read the thread and it didn’t seem to be reported, when we add some html code in After Header or Footer it doesn’t appear on the forum.

I may be the only one affected by this, i’m sorry if it is the case.

For example, here is my footer code :

<div style="text-align: center; margin: 2px 0 5px 0;"><span style="color:#9a9a9a; font-size:0.70rem; line-height:11px;"><i class="fa fa-twitter"></i> Suivre <a href="https://twitter.com/iunctisFR" title="Twitter @iunctisFR">@iunctisFR</a> sur Twitter - Ce site participe au Programme Partenaire amazon.fr - <a href="https://iunctis.fr/about" title="À propos de iunctis.fr">À propos</a></span></div>

4 Likes

I just tried adding this to my local dev instance and can confirm that I cannot see it After Header or Footer.