[dev] [commits] Horde branch master updated. 89bca7f749db1780dca80363d6f3d8c89f899df4

[Jan Schneider]

Zitat von Michael M Slusarz <slusarz at horde.org>:

> Quoting Jan Schneider <jan at horde.org>:
>
>> Zitat von Chuck Hagenbuch <chuck at horde.org>:
>>
>>> Quoting Michael M Slusarz <slusarz at horde.org>:
>
> [snip]
>
>>>> 2.) For themes that want to override the default graphics,  
>>>> require a themed_graphics.php file.  This file would contain a  
>>>> list of graphics that override the default graphics, e.g.
>>>>
>>>> $override = array(
>>>>  'graphic1' => 1,
>>>>  'graphic2' => 1
>>>>  ...
>>>> );
>>>>
>>>> This file could easily be auto-generated via a script (most  
>>>> likely using RecursiveIteratorIterator/RecursiveDirectoryIterator  
>>>> - see framework/bin/install_dev).
>>>>
>>>> This, to me, provides a cleaner solution than the  
>>>> 'themed_graphics' empty file for a variety of reasons:
>>>>
>>>> * To me, it *is* annoying to make sure that all default graphics  
>>>> are copied over to a theme.  This increases maintenance costs -  
>>>> to add a graphic to an application, a developer must remember to  
>>>> copy this graphic over to ALL themes or else each theme with  
>>>> graphics will be broken.  With the new technique, themes will  
>>>> continue to work if a new graphic is added. (albeit with the  
>>>> default, non-themed graphics).  Thus, maintenance of a theme  
>>>> falls on the theme creator/maintainer rather than a developer.
>>
>> But shouldn't we make it easier rather than harder for designers to  
>> create a new theme? At the moment all they have to do is to copy  
>> the graphics directory and then start overwriting the icons one by  
>> one as they go ahead with their theme.
>
> I'm still not persuaded by the argument that the proposed new method  
> is any harder than the previous method.  As an example, here's the  
> minimum needed to create a new theme:
>
> Old method:
> mkdir theme/
> mkdir theme/graphics
> cp graphics/* theme/graphics/*
>
> New method:
> mkdir theme/ [this could possibly be automated with the below command]
> create_theme.php theme

That's the culprit. Most designers probably don't have any means to  
run PHP scripts on the shell. They would either have to ask the admin  
each time they add another icon, or edit the PHP script with the icon  
definitions manually.

> Both of these cases will create a new theme that still uses all the  
> graphics/formatting in the previous theme.  I could also see the  
> 'create_theme.php' script being able to create the basic framework  
> for a new theme, so it would arguably be easier than the previous  
> manual method.

Using that script for creating a theme framework is fine. But you  
earlier suggested that this script would also create the PHP hash that  
contains all icons that are actually overriden by the theme. And I  
think this hurdle is too high.

>>>> This is even a stronger argument when thinking about a  
>>>> locally-maintained theme and updating an application.  If  
>>>> updating from Foo 1.0 to Foo 1.0.1, and Foo 1.0.1 adds several  
>>>> new graphics, the locally-maintained theme will ALWAYS break and  
>>>> there's nothing we (as Horde developers) can do about this.  This  
>>>> isn't an issue with the new theme - the local admin can determine  
>>>> at their leisure whether they need to introduce a new graphic.   
>>>> This will often be the case because 1) Horde app updates might be  
>>>> time-critical to fix security issues and 2) it may take awhile to  
>>>> locate/design/tweak a graphic to fit into a theme.
>
> To me, this is still the strongest argument for the new method.   
> There can be no doubt that the current method does NOT favor  
> maintainability - any update to the base theme requires alteration  
> of all themes or else layout may be broken.  The new method will  
> require no changes to ensure the theme will continue to work.
>
> Since one of our goals is to make updating between minor versions  
> easier (something that hopefully should be aided by the creation of  
> the SystemTasks framework), themes should be no different.

I agree with that, that's really a strong point.

>>>> * As mentioned previously, this method makes it easier to  
>>>> determine which graphics are being overridden.
>>>>
>>>> * Granted it is a minor point, but it does reduce the size of  
>>>> distribution files
>>>>
>>>> * PHP overhead is minimal - this method requires only that a  
>>>> simple PHP array be parsed/read into memory.
>>>>
>>>> * This is the only practical way of allowing view-specific  
>>>> portions of a theme (i.e. dimp theme) to override the defaults.   
>>>> For example, maybe we want a smaller/different graphic to be  
>>>> displayed when viewing mimp.  We also want this graphic to be  
>>>> used regardless of the theme.  This new method would allow the  
>>>> mimp view to override a default graphic.  This isn't possible  
>>>> with the old method.
>>>
>>> I'm still not entirely convinced on not copying graphics into  
>>> themes, but I'm okay with this method overall and your last  
>>> argument is particularly convincing.
>>
>> I agree that this a good argument, though I wonder why this would  
>> be necessary. Are the app views picking the icons by some  
>> automagical method?
>
> If you consider "auto-magical" to mean an extra parameter to  
> Horde_Themes::includeStylesheetFiles(), then yes :)  This is the  
> entire purpose of the 'sub' parameter to includeStylesheetFiles.   
> Sub-themes are meant to be literally "themes inside of themes".
>
> The idea is as follows: an installation wants to create a custom  
> theme.  This custom theme should also be allowed to override the  
> sub-themes.  And any given sub-theme can identify that it should be  
> the *exclusive* view for that given theme - i.e. not incorporating  
> *any* elements from the base theme or Horde theme.  If some  
> installation wanted to define an app's entire style, this is  
> possible using the new method and not possible using the old method.

But this has nothing to do with the icon sets unless I'm missing  
something. Icons defined in the CSS files are local to theme anyway.  
There is no problem with that, only with icons added through  
Horde::img/Horde_Themes_Image.

>> AFAIC the app views are using different view, even different  
>> controllers, and that's the place where the icons are included  
>> anyway. So they don't have to rely on some theme-loading magic to  
>> insert the right icon, they can simply point to the correct one.
>
> Not necessarily.  An example in imp/mimp: the default notification  
> code outputs an image for the notices.  In MIMP, by default, we  
> don't want to show this image so we can use CSS to hide it.   
> However, an installation may want to create a more involved UI for  
> mimp that better displays on smartphones.  A specific sub-theme  
> allows this to happen.

This is actually a bad example, because the theming of the  
notifications should really be done in CSS only. The notification  
should only output the ULs/LIs, not any embedded images.

> Fleshing this out a bit more, I see a theme definitions script  
> (theme.php) containing the following:
>
> $label = (string) Theme label to use in prefs.php (only needed in  
> horde theme)
> $graphics = (array) The list of graphics to override over the base app theme
> $exclusive = (string) Should this theme be the exclusive theme  
> definition?  If false, will follow the base override definitions
>
> Theme precedence:
> CSS files: Horde CSS, Horde theme CSS (if set), App CSS, App theme  
> CSS[, App subtheme CSS]
>    [Any theme may stop cascade by having $exclusive set]

I still don't completely follow why the $exclusive is required for  
sub-themes. Can explain this again with a real-world example?

> graphics/sounds: [App subtheme, ] App theme, App
>    [Any portion of the theme may stop cascade by having $exclusive set]

This doesn't look right to me, why do we need the reverse order here?

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/