ElkArte Community

Elk Development => Bug Reports => Exterminated Bugs => Topic started by: scripple on March 28, 2014, 04:17:43 pm

Title: Editor Theming Not Possible for Variants
Post by: scripple on March 28, 2014, 04:17:43 pm
Moving(-ish) this to bug reports at Ant's suggestion.  Basically the css file to style the wysiwyg editor is hard coded to use the file from the default theme so variants can't style the wysiwyg editor.  Making a dark variant I have definitely hit this problem.

A snippet from the original thread in the theme area.

Here's where the css for styling the wysiwyg editor iframe is supplied to sceditor.  It could certainly be set up where it checks the variant for a local css file and not finding it uses the default.  Or simply just require a variant to have a version of this css file.

Code: (GenericControls.template.php) [Select]
 $("#', $editor_id, '").sceditor({
 style: "', $settings['default_theme_url'], '/css/jquery.sceditor.elk.css",

So that's why a changes the custom.css doesn't help.  It's not being passed to that iframe.

My suggestion would be that variants have a required fourth file to theme the the editor.  I guess it would be called jquery.sceditor.elk_{variant}.css or something like that to be like the current three required files.

Then the php above that hands over the css to sceditor can do and isset($settings[variantish stuff]) or $context, whichever is appropriate to load the variant one instead.

Alternatively you could do the file exists check like you do for the custom.css to see if a variant has a custom sceditor file if you don't want to require that variants that don't touch that file have a copy.

I would not suggest passing the existing custom.css file into the sceditor though, as all the settings on body and such in custom.css are probably not what you'd want in the editor iframe.

All the other elements of the editor like the bbc buttons and such can be styled like anything else in the variant so they are not a problem.

Adding the fourth file wouldn't be very complicated as for any existing variant that was working without this change all you have to do is copy it from the theme itself.
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on March 28, 2014, 04:24:07 pm
Heh.  Ooops forgot to actually set the bug reports board in the drop down when I used the create new topic options. :P  Thanks for catching that for me.  :)
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on March 28, 2014, 04:32:13 pm
It happens frequently to me as well... lol
Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on March 28, 2014, 09:03:05 pm
GitHub issue here: https://github.com/elkarte/Elkarte/issues/586
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on March 29, 2014, 01:52:14 am
Here's a fix that shouldn't break anything you have now as it does a file exists check just like Load.php does for the custom.css file.  If it exists it sends the custom file from the variant to sceditor.  Otherwise it sends the one from the theme's main css directory.  I also replaced the default_theme references with just theme.  I figure that's better for people who want to use the default theme as a starting point for a new one.  If only the default theme is installed the two variables are equal anyway.

I chose a naming convention of jquery.sceditor.elk{variant}.css which for the light them would be jquery.sceditor.elk_light.css.

Here is the change in linux patch format.

Code: [Select]
--- GenericControls.template.php	2014-03-28 17:44:09.481051383 -0700
+++ GenericControls_WithVariant.template.php 2014-03-28 17:46:14.870132650 -0700
@@ -28,6 +28,16 @@
 
  $editor_context = &$context['controls']['richedit'][$editor_id];
 
+ // Load a variant specific css file for the WYSIWYG Editor?
+ if (!empty($context['theme_variant']) && file_exists($settings['theme_dir'] . '/css/' . $context['theme_variant'] . '/jquery.sceditor.elk' . $context['theme_variant'] . '.css'))
+ {
+ $editor_style = $settings['theme_url'] . '/css/' . $context['theme_variant_url'] . 'jquery.sceditor.elk' . $context['theme_variant'] . '.css';
+ }
+ else
+ {
+ $editor_style = $settings['theme_url'] . '/css/jquery.sceditor.elk.css';
+ }
+
  echo '
  <textarea class="editor', isset($context['post_error']['errors']['no_message']) || isset($context['post_error']['errors']['long_message']) ? ' border_error' : '', '" name="', $editor_id, '" id="', $editor_id, '" tabindex="', $context['tabindex']++, '" style="width:', $editor_context['width'], ';height: ', $editor_context['height'], ';" required="required">', $editor_context['value'], '</textarea>
  <input type="hidden" name="', $editor_id, '_mode" id="', $editor_id, '_mode" value="0" />
@@ -37,7 +47,7 @@
  $(document).ready(function(){',
  !empty($context['bbcodes_handlers']) ? $context['bbcodes_handlers'] : '', '
  $("#', $editor_id, '").sceditor({
- style: "', $settings['default_theme_url'], '/css/jquery.sceditor.elk.css",
+ style: "', $editor_style, '",
  width: "100%",
  height: "100%",
  resizeWidth: false,
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on March 29, 2014, 02:34:20 am
And here's a patch for the other minor issue I mentioned.  The remove format button on sceditor should really be grouped with the font,size,color buttons as that's what it removes.  This mode moves it from the end of row 2 to being grouped with those buttons.

Code: [Select]
--- Editor.subs.php	2014-03-28 18:32:00.261297960 -0700
+++ Editor.subs.php.mod 2014-03-28 18:31:51.617196160 -0700
@@ -231,9 +231,14 @@
  'space',
  'left', 'center', 'right', 'pre', 'tt',
  'space',
- 'font', 'size', 'color',
- 'space',
- );
+ 'font', 'size', 'color');
+
+ // Show the wysiwyg clear format button?
+ if (empty($modSettings['disable_wysiwyg']))
+ $context['bbc_tags']['row1'][] = 'removeformat';
+
+ $context['bbc_tags']['row1'][] = 'space';
+
  $context['bbc_tags']['row2'] = array(
  'quote', 'code', 'table',
  'space',
@@ -248,11 +253,10 @@
  // Allow mods to add BBC buttons to the toolbar, actions are defined in the JS
  call_integration_hook('integrate_bbc_buttons');
 
- // Show the wysiwyg format and toggle buttons?
+ // Show the wysiwyg toggle button?
  if (empty($modSettings['disable_wysiwyg']))
  {
  $context['bbc_tags']['row2'] = array_merge($context['bbc_tags']['row2'], array(
- 'removeformat',
  'source',
  'space',
  ));
Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on March 29, 2014, 08:18:27 am
I'm just wondering if it wouldn't be better to just serve the usual variant.css file after the jquery.sceditor.elk.css. That should keep things easier as far as customising goes, because the variant css would just need to contain any style overrides for the wysiwyg. That should be easier than trying to customise an entire editor file for each variant (I think).

Only catch is that it'd require a way to load both files sequentially. That's easy to do for old-style custom theming, where you just stacked up the file calls in <head>, but may be difficult with these files having to be called inside the script tags. Can this be done?
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on March 29, 2014, 02:54:02 pm
sceditor takes a "style" argument that tells it which style sheet to load.  I'm not sure if it can be given a list or not.
Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on March 29, 2014, 09:26:20 pm
Yeah that's what I was worried about. My js is pretty limited.
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on March 29, 2014, 09:35:40 pm
Could you look at the style sheet handed off to the editor and see if it even makes sense to try and combine with the rest of the css?  It starts of styling body and the body for the editor is rather different than the rest of the page.
Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on March 29, 2014, 09:36:47 pm
Good point. Hadn't given it much thought yet.
Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on March 29, 2014, 11:18:31 pm
Ok, took a look at the jquery.sceditor.elk.css. It's actually a pretty simple file, so yeah I reckon your solution is probably the best.

I was a bit worried that it might be the sort of file that'd bambozzle beginners, but after looking at it I don't think they'd have any problem with it. I was probably thinking of the base jquery.sceditor.css, which is more complex.
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on March 31, 2014, 12:28:39 am
Looking at the patch here:
http://www.elkarte.net/community/index.php?topic=1277.msg8647#msg8647
TBH, I would just load the variant file and that's all (that means if the theme doesn't have variants is just the "plain" css by default), otherwise I think it would be a bit of a mess (there may be two files and one is not loaded and so on).
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on March 31, 2014, 01:09:21 am
I also would prefer that you not do a file check and just load the one from the variant thus making it a required variant file.  But I didn't get any opinions from the elk crew about requiring another file in the variants directory so I went ahead and suggested a version that would not alter your current layout.
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on March 31, 2014, 06:47:59 pm
Okay, then let's do it "variants-only". ;D

Then while reading your second patch I started wondering: does it (still) make sense the option to disable the WYSIWYG editor?

ETA: I think this can be considered fixed too, am I wrong?
Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on March 31, 2014, 09:21:21 pm
Yes. Yes yes yes yes yes yes yes yes. Also yes. :D

Well, at least if you mean the option to use BBC mode instead of wysiwyg mode in SCEditor.

If you mean the option to not use SCEditor in quick reply, then I'd say probably no.
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on March 31, 2014, 09:49:34 pm
I mean this:
Quote
$helptxt['disable_wysiwyg'] = 'This setting disallows all users from using the WYSIWYG (&quot;What You See Is What You Get&quot;) editor on the post page.';
Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on March 31, 2014, 10:11:57 pm
Oh that one. Iz in admin, yes? Yeah that's silly IMO. Relic from the Dark Ages.
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on March 31, 2014, 11:14:06 pm
https://github.com/emanuele45/Dialogo/commit/dce3f306e420bbc9ba2612467c637c0f6fef63e8
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on March 31, 2014, 11:41:52 pm
https://github.com/emanuele45/Dialogo/commit/08194768aaba1d4a6547b7a72479e956328c8e62
I also had to tweak a bit the php code here:
https://github.com/emanuele45/Dialogo/commit/a8cf3efe8b18d045ecf88a73d68eeefb086a86c7
@Spuds what do you think?

There is still a small issue: the editor doesn't know if the bbcodes are disabled, so adding bbcode and toggling source/code views you may end up with formatting in the wysiwyg that will not be in the final post.
/me feels this is for 1.1. O:-)

ETA: replaced the second commit that was broken. :P
Title: Re: Editor Theming Not Possible for Variants
Post by: Spuds on March 31, 2014, 11:55:43 pm
I don't mind loosing the turn off wizzy option at all  :) , I don't think having that option saves anything with the new editor.

Does the call_integration_hook('integrate_bbc_buttons'); still work?  I've never tried modifying a static via a hook?
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on March 31, 2014, 11:58:37 pm
Good point!... I forgot about it... lol

https://github.com/emanuele45/Dialogo/commit/ff0e01036d8b1481363e00d3ef6f84fa5b28e5c5

In the meantime I also found some old markup and stuff:
https://github.com/emanuele45/Dialogo/commit/dc2760fe903365a858a727cb2ab64c9d4e85d2ed
and dropped it. O:-)
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on April 01, 2014, 01:59:52 am
A few things.

In the first mod
Code: [Select]
style: "', $settings['default_theme_url'], '/css/', $context['theme_variant_url'], 'jquery.sceditor.elk', $context['theme_variant'], '.css",

Why use $settings['default_theme_url']?  Shouldn't you just use $context['theme_url'] so that themes other than the default can load their own styling?

And on the buttons mod, if you're dropping the disable wysiwyg won't you always have the removeformat and source buttons?  There's no reason to have them done separately now.

Also, please put the remove format with the array('font', 'size', 'color') like array('font', 'size', 'color','removeformat')

It's really designed to go with those three buttons.  It's job is to turn those settings off.  See the example running on the sceditor site.
Title: Re: Editor Theming Not Possible for Variants
Post by: Spuds on April 01, 2014, 03:04:18 am
Quote
It's really designed to go with those three buttons.  It's job is to turn those settings off.  See the example running on the sceditor site.

It can also remove bold, italic, underline, strike through, sup and sub ... FWIW

Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on April 01, 2014, 04:38:17 am
Not to mention the left, right and centre justifying.....

I said not to mention them. :P

TBH I think the current location makes sense. BBC/WYSIWYG switch changes formatting of the form content, so having it next to that is as logical as anything else. It's just a matter of knowing wherre the button is. Once you've used it once, no problem.
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on April 01, 2014, 05:28:43 am
Yes, but those click on click off with highlighting.   Colors/Fonts/Size don't.  So if want center to end I click left or right.  If I want bold to end I click bold again.  If I want red text to end what do I do?  Click gray text?  Well, that kinda works but not really.  It makes it gray, not default, which might not be gray if you have multiple themes.   So you have to click the remove format.

It's a pretty awful design honestly.  Clicking the color box and not selecting a color sometimes clears the color, often not.   Bleh.
Title: Re: Editor Theming Not Possible for Variants
Post by: Spuds on April 01, 2014, 02:12:29 pm
I'd be more than happy to remove font, size and color from the toolbar as well, removing bbc codes is one of my favorite things to do as I think almost all of them are useless TBH.
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on April 01, 2014, 04:57:19 pm
I'm not sure that would make most people too happy.  I think in the end there's a hook to rearrange them however anyway, so it's ok if you insist on stranding the poor removeformat button from it's obvious stylistic cousin "A" brothers that's ok.    (But still wrong.   :P )
Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on April 01, 2014, 09:18:13 pm
TBH the font tag is useless, since it wont do a font stack that will show on any OS. It basically only does old Windows fonts, so anyone on Mac, Linux or mobile will probably just see the default theme font. I could happily lose the font button.

Size is useful for various things (a 12 pt heading works well in posts/articles that need a heading).

I've already mentoned that I disable coloured font on any forum I run, just to keep the nutters under some control.
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on April 03, 2014, 09:19:03 pm
A nice idea (for someone that want to mess with addons) could be a way to re-organize the editor's buttons! :D

/me has no time... :'(
Title: Re: Editor Theming Not Possible for Variants
Post by: Antechinus on April 03, 2014, 09:37:24 pm
Easy. Just hack the template. :P
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on April 07, 2014, 11:44:49 am
I start missing pieces and mixing stuff.
If I'm not wrong, apart from the position of the clear format icon, the rest is done in the current latest HEAD on master, right?.
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on April 12, 2014, 11:45:34 pm
This isn't actually exterminated.  The current master still won't let variants style the non-wiz editor.
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on April 13, 2014, 12:07:34 am
https://github.com/elkarte/Elkarte/blob/master/sources/subs/Editor.subs.php#L151

???
Title: Re: Editor Theming Not Possible for Variants
Post by: scripple on April 13, 2014, 12:42:54 am
Ooops.  I misremembered the naming scheme you chose.  Sorry.  I is stoopid.  :(
Title: Re: Editor Theming Not Possible for Variants
Post by: emanuele on April 13, 2014, 12:43:58 am
No problem, better a wrong review than a bug. :P