Merging style components #625

cyberalien · 2012-03-15T19:20:17Z

Topic: http://area51.phpbb.com/phpBB/viewtopic.php?f=108&t=42437
Ticket: http://tracker.phpbb.com/browse/PHPBB3-10632

Moving includes/template/ to includes/style/, adding template_ prefix to classes that deal only with templates PHPBB3-10632

Changing template classes prefixes from phpbb_template to phpbb_style (for classes that will work with styles) or phpbb_style_template (for classes that are specific to templates) PHPBB3-10632

Removing theme and template tables, adding new columns to styles table, deleting acp modules, deleting code that updates theme in updater PHPBB3-10632

…lates Implementing possibility of unlimited levels of parent templates. Paths are stored in style_parent_tree, entries are separated by / PHPBB3-10632

Removing theme and template entries in all files, except for acp styles section PHPBB3-10632

Updating template inheritance section in coding guidelines PHPBB3-10632

Removing theme.cfg and template.cfg PHPBB3-10632

…le.cfg Adding template data to style.cfg and removing obsolete comments PHPBB3-10632

Adding background colors for row iterations and font color for disabled rows to admin control panel css. PHPBB3-10632

Adjusting unit tests for new styles table structure PHPBB3-10632

Adding new language variables for acp_styles and removing some unused variables PHPBB3-10632

New acp_styles, completely rewritten PHPBB3-10632

New acp_styles.html, completely rewritten PHPBB3-10632

Creating phpbb_style class, changing template initialization to style initialization PHPBB3-10632

Changing $style to $style_id in user::setup to avoid conflict with new global style variable PHPBB3-10632

Renaming style locator to style resource locator PHPBB3-10632

Changing set_templates() to set_style() and removing second parameter, changing get_main_template_path() to get_main_style_path(), removing template_root_for_style(), updating docblocks PHPBB3-10632

Changing "template" to "style" in all functions that deal with styles, changing error messages, updating docblocks PHPBB3-10632

Removing functions that are now handled by phpbb_style class, allowing to write $context, updating docblocks PHPBB3-10632

Moving functions that deal with styles from template to style class, updating docblocks PHPBB3-10632

Changing template initialization to style initialization. PHPBB3-10632

Updating code in test cases for new template classes. PHPBB3-10632

cyberalien · 2012-03-24T10:02:28Z

This PR is ready for merge and is blocking development of more features.

Few notes:

acp_styles is not complete, it is missing files editor. Editor will be added in another pull request
acp_styles purges all cache when something is done that requires templates cache update. See PE Allow all admins to purge cache #582

So don't really mind current acp_styles, it isn't complete and will be altered in further pull requests.

…style-components * upstream/develop: (65 commits) [ticket/10730] Added label tag around "select" text in post splitting UI [ticket/10732] Add config_dev.php and config_test.php to .gitignore [ticket/10586] Added space in if statement [ticket/10586] Tidy up comments [task/php5.3] Updated range of tested PHP versions [task/php5.3] Looks like I missed a few places that needed PHP 5.2 changed to PHP 5.3.2 [task/php5.3] Changed minimum PHP requirement for Ascraeus to 5.3.2 [ticket/10723] Stop Travis running all tests on sqlite [ticket/10703] Added a condition to check if ext directory exists [task/travis] Refactor php version check for dbunit install [task/travis] Exclude functional and slow tests [ticket/10719] Revert "Skip functional tests on PHP 5.2" [task/travis-develop2] Update version from 5.3 to 5.3.2 [task/travis] Dropping support for 5.2 in develop branch [task/travis] Some more small travis fixes [task/travis] Rename travis phpunit config files [task/travis] Fixing some travis issues [ticket/10684] Adjust function and parameter name, minor changes. [task/travis] Add automated testing to readme [task/travis] Removing development information ... Conflicts: phpBB/install/database_update.php

cyberalien · 2012-03-29T19:39:04Z

Merged latest develop in it and resolved merge conflict. This PR is ready, so whenever someone has time, please take a look at it.

cyberalien · 2012-03-30T09:00:18Z

This is a rather large pull request, it includes many changes and is required in order to implement several other RFCs, therefore it would be great to validate it as fast as possible. To make validation easier, here is full list of changes and justification of those changes.

Database changes.

Style components no longer exists, so tables for theme and template become useless. I've removed them. From styles table I've removed theme_id and template_id because they no longer exist and added the following fields:

bbcode_bitfield - bitfield for bbcode parsing. It used to be in tempaltes table.
style_path - name of directory where style is stored, like "prosilver". This field used to be in template and theme tables as template_path and theme_path.
style_parent_id - id of parent style. This field used to be in template table as template_inherits_id.
style_parent_tree - tree of parent styles. Unlike old field template_inherit_path from templates table, this field is much longer, allowing to store many paths. Field stores full list of parent styles, separated with "/". For example, "prosilver/progreen"

Why was template inheritance renamed to styles tree? Old term was rather confusing to not only users, but to some style authors. It is essentially a tree, therefore styles tree makes much more sense and is easier to understand.

Updating from 3.0 to 3.1 invalidates all old styles, styles table must be cleared during update, so instead of updating data in styles table, I've added code to purge old styles table and insert prosilver data instead.

Test cases were updated to match new database structure.

Style configuration file changes.

Data from old tempalte.cfg was moved to style.cfg. "template_bitfield" stayed the same, "inherit_from" was renamed to "parent" that points to parent style's name.

Style classes changes.

Templates are no longer separate from styles, therefore instead of initializing templates, template system now initializes styles. Functionality of some template classes was changed a bit and all classes were renamed. All template classes were moved from includes/template/ to includes/style/

I've extracted superclass from template, separating template and style functionality. Main class is no longer phpbb_template, but phpbb_style. It is a new class that is responsible for initializing style. It creates all necessary classes, sets them up, including new template class. Variable $style for it is created in common.php. Styles class has fewer parameters than template class used to have because it creates necessary sub classes in constructor instead of requiring to create them in phpBB code.

Template class is now called phpbb_style_template. Scripts no longer use it to set custom template paths, instead phpbb_style class does that. Template class is responsible for dealing with templates: compiling, showing, storing variables. All those functions work like they used to, but with adjustments for new structure.

Template locator class is now called phpbb_style_resource_locator. It used to store only paths to templates, but now it stores paths to styles and reference to templates directory inside styles directory. Why was it changed? To make it usable for other style resources, allowing implementation of features like style specific language variables, INCLUDEJS tag and in future maybe expanding styles tree to css files.

Path provider classes were also renamed. They now locate style directories instead of template directories. Their ckass name prefixes were changed from "phpbb_template_" to "phpbb_style_"

Template compiler, renderer, context and filter classes weren't changed. They perform same task as they used to, but their names now start with "phpbb_style_template_" instead of "phpbb_template_".

Style initialization changes.

Scripts no longer initialize template, they initialize style instead. Function $template->set_ext_dir_prefix() moved from $template to $style, function $template->set_custom_template() no longer exists and has been replaced by $style->set_custom_style() that has different parameters list.

Old function set_custom_template() did not allow multiple parent styles. New function set_custom_style() does. Format: set_custom_style($name, $paths, $template_path);

First parameter is name of style, same as second parameter was for set_custom_template(). It defines variable for cached templates. Installer and admin control panel set it to "admin", otherwise it is directory name of current style, like "prosilver".

Second parameter is array of style paths. If there is only one style path, second parameter can be a string to make code easier to read. First element of array is main style's path, second is its parent style's path, then parent of parent style and so on.

Third parameter is string, pointing to location of template files relative to style directory. For acp and installed its empty string, for other pages its "template/". This is optional parameter. If false or doesn't exist, set_custom_style() will ignore it and keep old value.

You can see code changes by looking at phpBB/adm/index.php diff

Admin control panel changes.

Old acp_styles is no longer functional. Changing it requires rebuilding 90% of its code, so instead I've just deleted it and created new styles management. It is not complete yet. Current implementation creates only basic functionality and doesn't clean up language variables that are no longer used. The rest of changes will be available in separate pull requests.

To display styles tree, I've added several new row classes to admin control panel's css file as well as classes for inactive rows.

imkingdavid · 2012-03-30T12:51:48Z

phpBB/docs/coding-guidelines.html

-        # Defining a different template bitfield
-        template_bitfield = lNg=
+# Defining a different template bitfield
+# template_bitfield = lNg=


Is this line supposed to have a # at the start?

Yes, its commented out by default. Style author can enable it to override default value

Changing acp styles welcome message a little bit. PHPBB3-10632

michaelcullum · 2012-03-30T13:17:36Z

phpBB/docs/coding-guidelines.html

-        name = inherits
-        copyright = &copy; phpBB Group, 2007
-        version = 3.0.3
+# General Information about this style


Why the change in indenting?

It was a bug in old document.

michaelcullum · 2012-03-30T13:18:39Z

phpBB/includes/acp/info/acp_styles.php

@@ -17,11 +17,11 @@ function module()
 		return array(
 			'filename'	=> 'acp_styles',
 			'title'		=> 'ACP_CAT_STYLES',
-			'version'	=> '1.0.0',
+			'version'	=> '2.0.0',


Is this necessary? And for now it should probably be 1.1.0 until you do the rewrite?

I'm not sure about that. It is a full rewrite, so probably changing first number would make more sense.

* develop: (175 commits) [feature/ajax] Remove strange non-breaking spaces from approve button [feature/ajax] Add entirely unrelated but nice newlines [feature/ajax] Unify phpbb_json_response instantiation [feature/ajax] Fix acp_styles activate_deactivate ajax callback name [feature/ajax] Send correct activate/deactivate JSON response in acp_profile [ticket/10270] Alter background colors for posts [feature/ajax] Remove not working module enable/disable ajax code [feature/ajax] Replace static call to phpbb_request with OO [feature/ajax] Remove quick-reply AJAX handling until we have something good [ticket/10270] Changing close button for ajax popups [ticket/10270] Disabling links in disappearing content [ticket/10291] Fixed an AJAX bug on quick reply form submit. [ticket/10273] Fixed accepting / denying posts AJAX. [ticket/10272] Removed code that was prevent event propogation in AJAX. [ticket/10291] Fixed a bug in the quick reply AJAX. [feature/ajax] Handle acp_modules error cases with JSON response [feature/ajax] Fix filter check, quick mod tools data-attribute [feature/ajax] Use the error handler [feature/ajax] Generic error handling with a phpbb.alert box [feature/ajax] Change filter semantics, some minor adjustments ... Conflicts: phpBB/adm/style/acp_styles.html phpBB/includes/acp/acp_styles.php

cyberalien · 2012-03-31T07:44:38Z

Updated to latest develop.

igorw · 2012-03-31T12:26:01Z

phpBB/includes/style/style.php

+		{
+			$this->provider = new phpbb_style_extension_path_provider($phpbb_extension_manager, $this->provider);
+		}
+		$this->template = new phpbb_style_template($this->phpbb_root_path, $this->phpEx, $this->config, $this->user, $this->locator, $this->provider);


I don't like the fact that phpbb_style is creating its own dependencies. They should be injected.

Instead of false for optional values, null should be used. False implies a boolean, null is clearer in that regard.

So, move out provider and locator out of constructor and initialize them separately, then pass as parameters to style's constructor?

Yes, this decouples phpbb_style from the implementation of the other ones, allowing them to be replaced. Particularly useful when testing, but in general simply more flexible.

igorw · 2012-03-31T12:30:39Z

docs/coding-guidelines.html needs to be updated, it still has some mentions of themes.

So does:

docs/hook_system.html

igorw · 2012-03-31T12:34:01Z

We should probarly get rid of $user->theme, but that can also be done in a separate PR.

Updating styles section in coding guidelines PHPBB3-10632

cyberalien · 2012-03-31T13:37:33Z

Coding guidelines: updated

Hooks: I don't see anything in that file is changed by this PR

$user->theme: it would make sense to update it, but yep, it would be better to do in separate PR to keep this one as small as possible

igorw · 2012-03-31T13:39:38Z

You're right, we still have T_THEME_PATH, etc.

PHPBB3-10632

Fixing notices and usability issues PHPBB3-10632

…separately Moving locator and path provider initialization out of style class PHPBB3-10632

…or styles Changing from "delete" to "uninstall" in acp_styles to avoid confusing users PHPBB3-10632

… of style Moving template initialization out of style constructor PHPBB3-10632

Fixing typo in template unit tests PHPBB3-10632

When on install page and all styles are installed, fix back link to go to main styles page. PHPBB3-10632

naderman · 2012-04-02T19:55:51Z

phpBB/adm/index.php

-$template->set_ext_dir_prefix('adm/');
-$template->set_custom_template($phpbb_admin_path . 'style', 'admin');
+// Set custom style for admin area
+$style->set_ext_dir_prefix('adm/');


$phpbb_style

cyberalien added 22 commits March 14, 2012 22:57

[feature/merging-style-components] Moving template classes to style

7d414d0

Moving includes/template/ to includes/style/, adding template_ prefix to classes that deal only with templates PHPBB3-10632

[feature/merging-style-components] Renaming template classes

b7d84a5

Changing template classes prefixes from phpbb_template to phpbb_style (for classes that will work with styles) or phpbb_style_template (for classes that are specific to templates) PHPBB3-10632

[feature/merging-style-components] Updating database and acp modules

ae3b0f7

Removing theme and template tables, adding new columns to styles table, deleting acp modules, deleting code that updates theme in updater PHPBB3-10632

[feature/merging-style-components] Implementing unlimited parent temp…

234e5d6

…lates Implementing possibility of unlimited levels of parent templates. Paths are stored in style_parent_tree, entries are separated by / PHPBB3-10632

[feature/merging-style-components] Updating PHP files

ae7d290

Removing theme and template entries in all files, except for acp styles section PHPBB3-10632

[feature/merging-style-components] Updating coding guidelines

68336ab

Updating template inheritance section in coding guidelines PHPBB3-10632

[feature/merging-style-components] Removing theme.cfg and template.cfg

1cc9f60

Removing theme.cfg and template.cfg PHPBB3-10632

[feature/merging-style-components] Adding template information to sty…

95a6963

…le.cfg Adding template data to style.cfg and removing obsolete comments PHPBB3-10632

[feature/merging-style-components] Admin.css additions for acp_styles

f32cc3a

Adding background colors for row iterations and font color for disabled rows to admin control panel css. PHPBB3-10632

[feature/merging-style-components] Adjusting unit tests

ce21565

Adjusting unit tests for new styles table structure PHPBB3-10632

[feature/merging-style-components] Adding new language variables

0c6955e

Adding new language variables for acp_styles and removing some unused variables PHPBB3-10632

[feature/merging-style-components] New acp_styles

e35a20f

New acp_styles, completely rewritten PHPBB3-10632

[feature/merging-style-components] New acp_styles template

39944a0

New acp_styles.html, completely rewritten PHPBB3-10632

[feature/merging-style-components] Creating style class

3997ffa

Creating phpbb_style class, changing template initialization to style initialization PHPBB3-10632

[feature/merging-style-components] Changing $style to $style_id

c83f386

Changing $style to $style_id in user::setup to avoid conflict with new global style variable PHPBB3-10632

[feature/merging-style-components] Renaming style locator

8b7c2c3

Renaming style locator to style resource locator PHPBB3-10632

[feature/merging-style-components] Changing path provider

c692e0d

Changing set_templates() to set_style() and removing second parameter, changing get_main_template_path() to get_main_style_path(), removing template_root_for_style(), updating docblocks PHPBB3-10632

[feature/merging-style-components] Changing resource locator

0b2abe5

Changing "template" to "style" in all functions that deal with styles, changing error messages, updating docblocks PHPBB3-10632

[feature/merging-style-components] Changing template class

5b149e9

Removing functions that are now handled by phpbb_style class, allowing to write $context, updating docblocks PHPBB3-10632

[feature/merging-style-components] Changing style class

1ce4d4c

Moving functions that deal with styles from template to style class, updating docblocks PHPBB3-10632

[feature/merging-style-components] Updating style initialization

fd96f97

Changing template initialization to style initialization. PHPBB3-10632

[feature/merging-style-components] Updating test cases

d25b607

Updating code in test cases for new template classes. PHPBB3-10632

imkingdavid reviewed Mar 30, 2012
View reviewed changes

[feature/merging-style-components] Changing acp_styles text

506951e

Changing acp styles welcome message a little bit. PHPBB3-10632

michaelcullum reviewed Mar 30, 2012
View reviewed changes

igorw reviewed Mar 31, 2012
View reviewed changes

[feature/merging-style-components] Updating styles in coding guidelines

e13e4d9

Updating styles section in coding guidelines PHPBB3-10632

igorw and others added 7 commits March 31, 2012 16:53

[feature/merging-style-components] Fix notices in acp_styles

bc46cfd

PHPBB3-10632

[feature/merging-style-components] Fixing few errors in acp_styles

9611997

Fixing notices and usability issues PHPBB3-10632

[feature/merging-style-components] Initializing locator and provider …

360312f

…separately Moving locator and path provider initialization out of style class PHPBB3-10632

[feature/merging-style-components] Renaming "delete" to "uninstall" f…

0540509

…or styles Changing from "delete" to "uninstall" in acp_styles to avoid confusing users PHPBB3-10632

[feature/merging-style-components] Moving template initialization out…

17989c1

… of style Moving template initialization out of style constructor PHPBB3-10632

[feature/merging-style-components] Fix for unit tests

377db78

Fixing typo in template unit tests PHPBB3-10632

[feature/merging-style-components] Fix back link on install page

5d07b16

When on install page and all styles are installed, fix back link to go to main styles page. PHPBB3-10632

igorw merged commit 5d07b16 into phpbb:develop Mar 31, 2012

naderman reviewed Apr 2, 2012
View reviewed changes

dhruvgoel92 mentioned this pull request Apr 9, 2013

[ticket/11485] add columns to update_schema before adding data #1326

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merging style components #625

Merging style components #625

cyberalien commented Mar 15, 2012

cyberalien commented Mar 24, 2012

cyberalien commented Mar 29, 2012

cyberalien commented Mar 30, 2012

imkingdavid Mar 30, 2012

cyberalien Mar 30, 2012

michaelcullum Mar 30, 2012

cyberalien Mar 30, 2012

michaelcullum Mar 30, 2012

cyberalien Mar 30, 2012

cyberalien commented Mar 31, 2012

igorw Mar 31, 2012

cyberalien Mar 31, 2012

igorw Mar 31, 2012

cyberalien Mar 31, 2012

igorw commented Mar 31, 2012

igorw commented Mar 31, 2012

cyberalien commented Mar 31, 2012

igorw commented Mar 31, 2012

naderman Apr 2, 2012

Merging style components #625

Merging style components #625

Conversation

cyberalien commented Mar 15, 2012

cyberalien commented Mar 24, 2012

cyberalien commented Mar 29, 2012

cyberalien commented Mar 30, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyberalien commented Mar 31, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igorw commented Mar 31, 2012

igorw commented Mar 31, 2012

cyberalien commented Mar 31, 2012

igorw commented Mar 31, 2012

Choose a reason for hiding this comment