Remove hard-coded copyright and make it a block

@oscarvargas102 Patch does not comply with Drupal coding standards, please correct!

The patch also does not do what @tkoleary original suggested.

Drupal coding standars fixed

Moving to Needs work, the patches so far are unrelated to the original issue. If you would like a configurable copyright block, you can always create a custom block and place it in the footer region (once this issue is Fixed).

@samuel.mortenson removing hard-coded copyright from twig is easy, adding a custom block with content via YML has proven a bit of a challenge.

Does anybody have any pointers as to how this could be achieved?

So far, I've tested the following block.block.zurb_foundation_copyright.yml in /config/install with the following contents:

id: zurb_foundation_copyright
plugin: block
langcode: es
status: true
theme: zurb_foundation
region: footer_middle
provider: zurb_foundation
settings:
  id: zurb_foundation_copyright_block
  label: 'Zurb Foundation Copyright'
  label_display: '0'
visibility: {  }

The block gets created and assigned to the correct theme/region, but every other block is cleared upon install and so far I can't figure out how to add block_content only using YMLs...

@tkoleary did you see this working somewhere else we could use as an example?

Thanks!

You cannot create Content Blocks in .yml files, as they're Content Entities. I'll wait to hear @tkoleary's opinion about the patches so far before proceeding.

I'm all for an interim commit that removes the hard coded values, a change we should strive for.

As for providing a block out of the box for this specific need, I am on the fence about. If it whittles down to just a block, I feel like most people are going to want full control of it anyway, and providing regions seems to be enough to me to grant that ability.

I would probably side with Kevin on this issue. At this point you're just asking to ship editable content out of the box.

Putting this back into review, if anyone would like to RTBC the most recent patch I would be willing to commit it to 8.x-6.x. @tkoleary's notes about a module-provided configurable copyright block would have to be addressed in a new project.

I rerolled the patch against 1a6cffb5dc95516a502f7cf28c77d9bef90d1004 by hand, and tested it in Drupal 8.0.6 standard install. it saves the Copyright settings correctly & I believe it is RTBC.

A few nitpicks/changes:

diff -u b/config/install/zurb_foundation.settings.yml b/config/install/zurb_foundation.settings.yml
--- b/config/install/zurb_foundation.settings.yml
+++ b/config/install/zurb_foundation.settings.yml
@@ -24,3 +24,4 @@
   zurb_foundation_page_account_info: true
-  block_copyright_check: true
+  zurb_foundation_block_copyright_custom_text: false
+  zurb_foundation_block_copyright_text: ''

No default value for the custom text, and the naming scheme of the variable doesn't match anything else.

diff -u b/templates/page.html.twig b/templates/page.html.twig
--- b/templates/page.html.twig
+++ b/templates/page.html.twig
@@ -196,19 +196,17 @@
           {% endif %}
         </footer>
       {% endif %}
-      {% if block_copyright %}
...

Checking for block_copyright here hid the normal copyright text unless a custom one was defined, which doesn't make sense. I removed this if statement and kept the normal if/else block.

+  $form['page_elements']['zurb_foundation_block_copyright_custom_text'] = [
     '#type' => 'checkbox',
-    '#title' => t('Use the default Copyright label'),
-    '#default_value' => theme_get_setting('block_copyright_check'),
-    '#tree' => FALSE,
-    '#description' => t('Uncheck here if you want modify the copyright label')
+    '#title' => t('Use custom copyright text'),
+    '#default_value' => theme_get_setting('zurb_foundation_block_copyright_custom_text'),
+    '#description' => t('Check this if you want modify the copyright text.')

Nitpick - I don't like double negatives in forms, and I would like the variable naming/form text here to match what we already have with custom back text.

diff -u b/zurb_foundation.theme b/zurb_foundation.theme
--- b/zurb_foundation.theme
+++ b/zurb_foundation.theme
@@ -337,12 +337,11 @@
   // Variable to disable hard-coded login elements.
   $variables['show_account_info'] = theme_get_setting('zurb_foundation_page_account_info');

-  // Copyright block settings
-  $block_copyright_value = theme_get_setting('block_copyright');
-  $variables['block_copyright'] = $block_copyright_value;
-
-  $block_copyright_check = theme_get_setting('block_copyright_check');
-  $variables['block_copyright_check'] = $block_copyright_check;
+  // Copyright block settings.
+  $variables['block_copyright'] = [
+    '#markup' => theme_get_setting('zurb_foundation_block_copyright_text')
+  ];
+  $variables['block_copyright_custom_text'] = theme_get_setting('zurb_foundation_block_copyright_custom_text');
 }

Rendered the custom copyright text with #markup, which protects against XSS vulnerabilities. As far as I know rendering the raw text in twig would have allowed <script> tags and the like.

Thanks I was wondering about sanitization of the string, not yet familiar with the D8 way. This seems good to me although I don't know if it makes sense to render "bottom-bar panel" even when there is no string going on (in the second chunk of the interdiff). The earlier version had a broader if statement to prevent that rendering. Otherwise RTBC?

This patch doesn't add an explicit way to remove the copyright - so we would have to do an if statement that checks if a custom copyright is used and that the custom copyright (string) is empty. I would consider conditionally rendering that a distinct setting, so we would need another setting saying something like "Hide copyright block". Someone could add that in this issue if it's needed or open a new issue after this is RTBC'd and committed.

Okay I learned some interesting things in this process. Question: Is it okay to not sanitize the boolean into twig?

I think this has the toggle feature we are looking for, I tested it and it seems to work correctly, with no markup shown if everything is disabled. Please take it for a spin.

@HongPong Using variables in evaluations like {% if block_copyright_show %} is totally fine, as long as you aren't actually printing them there shouldn't be a risk.

@HongPong

I think we should pick this back up. If you re-roll I can review it.

I rerolled this by hand so I don't have a interdiff. This should apply to the latest 8.x-6.x dev branch.

I tested it on Drupal 8.7.7 and it appears to work correctly. Marking it as ready for review so someone else can test it as well.

I tested the copyrightblock-2655760-20.patch in #20 and it works fine.

It does make the copyright go full width now, which is a change, but this seems fine as is. It removes two less unnecessary divs in page.html.twig:

-      <div class="bottom-bar callout secondary">
-        <div class="row">
-          <div class="large-12 columns">
-          &copy; {{ "now"|date('Y') }} {{ site_name }} {{ 'All rights reserved.'|t }}
-          </div>
+      {% if block_copyright_show %}
+        <div class="bottom-bar callout secondary large-12 columns">
+          {% if not block_copyright_custom_text %}
+            <p>&copy; {{ "now"|date('Y') }} {{ site_name }} {{ 'All rights reserved.'|t }}</p>
+          {% else %}
+            {{ block_copyright }}
+          {% endif %}

Thank you sim_1 and Taiger for revisiting and testing this. I think for now this is a good improvement. Wow that took quite a few years!