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

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
98bc2d1
Improving theme cloning process in order to minimize manual updates
shaal Jul 3, 2018
25998a8
fixing comment line
shaal Jul 3, 2018
1c572e3
remove a non-existing directory `components/css`
shaal Jul 5, 2018
4b7adf4
Support the `-slim` option
shaal Jul 5, 2018
06d72cb
Fixing the function _emulsify_alter_files (code by @ccjjmartin)
shaal Jul 5, 2018
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
View file
Open in desktop
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 •
edited
Loading

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
2 changes: 2 additions & 0 deletions emulsify.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,8 @@ function _emulsify_get_files_to_alter() {
'emulsify.theme',
'emulsify.breakpoints.yml',
'emulsify.libraries.yml',
'templates/navigation/menu--main.html.twig',
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like these two lines should be removed since they are already copied by default when the slim option is not used. See a few lines down where we copy the entire contents of the templates and components directories. This would add these files to the --slim option which is designed not to copy any files from these directories and be a bare bones install. Let me know if you were having some trouble with these files not showing up as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ccjjmartin, I was not aware of the --slim option.
When I run php emulsify.php -slim myslimtheme I am getting an error -

PHP Warning:  copy(/home/shaal/code/lando/rosenberg/web/themes/contrib/emulsify/components/css): failed to open stream: No such file or directory in /home/shaal/code/lando/rosenberg/web/themes/contrib/emulsify/emulsify.php on line 125
Failed on Phase: Copy files.Your theme was not successfully created.

I think the following line of code is trying to copy a directory that does not exist - components/css
https://github.com/fourkitchens/emulsify/blob/develop/emulsify.php#L495

'components/_patterns/01-atoms/04-images/icons/_icon.twig',
);
// If we would like to have a bare copy we use is slim option.
if (drush_get_option('slim') === TRUE) {
Expand Down