Skip to content
This repository has been archived by the owner on Mar 31, 2023. It is now read-only.

Improving theme cloning process in order to minimize manual updates #268

Merged
merged 5 commits into from
Jul 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions components/_patterns/01-atoms/04-images/icons/_icon.twig
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
*/
#}
{% set icon_base_class = icon_base_class|default('icon') %}
{# For Drupal: Update this path to match your theme directory structure #}
{# For Pattern Lab: The icon_url is defined in _data/paths.json. No action is required #}
{% set icon_url = icon_url|default('/themes/custom/YOURTHEME/dist/img/sprite/') %}
{# Update `emulsify` here to your theme's machine name #}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shaal I am curious as to the broken behavior you experienced here. Did "emulsify" on this line get replaced? On the line right after this did it not get replaced? I am definitely pro simplifying install processes so I would love to hear more about the bug you encountered.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the goal here is to set this as emulsify by default so that it can be auto-replaced. In the "before" code, it says to manually enter your theme's machine name. Setting it automatically on cloned theme creation is preferred.

Copy link
Collaborator

@ccjjmartin ccjjmartin Jul 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I was focused on the comment containing the string emulsify but that actually isn't the main change here. I should have been looking at the YOURTHEME versus emulsify. I am good with this change because it makes the out of the box experience better in most scenarios. I will point out that this is not going to work for people who don't put their theme in the root of /themes/ versus /themes/custom/ But that is likely a bigger issue that what we are trying to address here. We should consider replacing this (in another PR not this one) with just a string called PATH and then it drops in the theme path.

Alterations are defined in the _emulsify_get_alterations function which is currently only doing these:

    'Emulsify' => $human_readable_name,
    'emulsify' => $machine_name,
    'Theme that uses Pattern Lab v2' => $description,
    'hidden: true' => '',

It would be pretty easy to add a new line:

    'PATH' => $path,

And add another parameter for to that same function to pass the path in. When I wrote this I wasn't thinking we would need the PATH within the twig files.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created this ticket for follow up on this: #271

{# For Drupal: The icon_url is defined in the following line. No action is required #}
{% set icon_url = icon_url|default('/themes/custom/emulsify/dist/img/sprite/') %}
{{ attach_library('emulsify/sprite') }}

<svg {{ bem(icon_base_class, (icon_modifiers), icon_blockname, (icon_js_class)) }}>
Expand Down
13 changes: 6 additions & 7 deletions emulsify.php
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,6 @@ function _emulsify_get_files_to_copy() {
'components/_macros',
'components/_meta',
'components/_twig-components',
'components/css',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, this needed to go.

'components/images',
'components/_patterns/style.scss',
'components/_patterns/00-base/global/01-colors',
Expand Down Expand Up @@ -550,7 +549,7 @@ function _emulsify_get_files_to_rename() {
* @return boolean
* A boolean representing the success or failure of the function.
*/
function _emulsify_alter_files($theme_path, array $files_to_alter = array(), array $alterations = array(), $absolute = FALSE) {
function _emulsify_alter_files($theme_path, array $files_to_alter = array(), array $alterations = array(), $absolute = FALSE, int $depth = 0) {
if (empty($files_to_alter) || empty($alterations)) {
return TRUE;
}
Expand All @@ -568,11 +567,11 @@ function _emulsify_alter_files($theme_path, array $files_to_alter = array(), arr
$files = scandir($file_path);
$files = array_splice($files, 2);
foreach ($files as $file) {
$processed_files[] = $file_path . $file;
}
$alter_status = _emulsify_alter_files($theme_path, $processed_files, $alterations, TRUE);
if ($alter_status === FALSE) {
return FALSE;
$processed_file = [$file_path . DIRECTORY_SEPARATOR . $file];
$alter_status = _emulsify_alter_files($theme_path, $processed_file, $alterations, TRUE, $depth + 1);
if ($alter_status === FALSE) {
return FALSE;
}
}
}
elseif ($file_type === 'file') {
Expand Down