Remove duplicate hard coded logo and site name in theme

I noticed the same thing when I started work porting the theme, so I added a setting to remove each of the hard-coded page elements.

You can disable them by visiting /admin/appearance/settings/zurb_foundation, and expanding "Page Elements".

Is this an acceptable solution?

Let's consider this for 8.x-6.x, for all of our hard-coded block replacements.

Is this for sure going to be removed, in favor of the site branding block, going forward? I noticed the site_slogan variable isn't passed through even though it is being used in page.html.twig. I was going to work on a patch to bring it back in and as a configurable option (visibility), but if we plan to remove it, I won't bother with the patch.

Hi All,

There is duplicate code exist in the page.html.twig for branding of site, as we know we have a branding block in drupal 8. So we need to just enable that block in a particular region and we get a site logo and site name on that region. So we don't not need to add logo code in page.html.twig.If you want to add your own classes you can create a tpl called block--system-branding-block.html.twig where you can change the structure of logo and site name , slogan. So i submit patch to remove the duplicate code form the page.html.twig file.

Thanks
Sumit

Patch applied successfully,Thanks...

This patch is incomplete, as there are theme settings for showing/hiding the logo (zurb_foundation_page_site_logo) and preprocess logic that created a linked logo image. I would almost prefer to just make the default setting for "zurb_foundation_page_site_logo" in config/install/zurb_foundation.settings.yml to "false".

@samuel.mortenson Thanks for feedback. I have submit patch that you mention in your last comment #12.

@sumit, patch is apply success fully.

Remove hard code but in the appearance/settings/zurb_foundation option Page Elements show site name and site logo checkbox is appear (refer attached screen shot ZURBFoundation.png).
Is this acceptable or not, if not then remove this code:
$form['page_elements']['zurb_foundation_page_site_name'] = array(
'#type' => 'checkbox',
'#title' => t('Show site name'),
'#description' => t('Determines if the hard-coded site name should be displayed.'),
'#default_value' => theme_get_setting('zurb_foundation_page_site_name'),
);

$form['page_elements']['zurb_foundation_page_site_logo'] = array(
'#type' => 'checkbox',
'#title' => t('Show site logo'),
'#description' => t('Determines if the hard-coded site logo should be displayed.'),
'#default_value' => theme_get_setting('zurb_foundation_page_site_logo'),
);

@bandana Thanks for review. Added patch with new modification.

#15 patch is apply success fully and working fine.. Thanks.

@ tkoleary - please review @Sumit kumar and @bandanasharma patch - request for approval

Many thanks @tkoleary - request for a code review from anyone in the community ? Our Mr dependable @ Gábor Hojtsy ?

#12 says there is preprocess logic that created a linked logo image. Since no preprocess logic is removed in the patch, this sounds incomplete.

As for whether this is the right technical solution to what Zurb Foundation is aiming at, I am not the right person to tell :)

@Gober remove zurb_foundation_page_site_logo from preprocess logic

I have reviewed the #21 and i couldn't find any preprocess logic for site logo and site name.I think this is fine.

@tkoleary If we are not providing the functionality of site_logo and site_name, as these are in D8 core now, So what we have to do with it. Because I am still able to find the site_name variable and site_logo in documentation zurb_foundation.theme and inc/menu.inc

Are we going to remove it ? or we are just disabling it temporary from zurb_foundation.settings.yml ?

@samuel.mortenson Need your confirmation for Comment number #24

@sumit-kumar Per my comment in #12, it seems significantly easier to just disable the site logo and name in zurb_foundation.settings.yml than to remove the preprocess hooks, template definitions, theme settings, etc. This provides backwards compatibility with users who expect this functionality and only requires two lines of code to be changed.

patch changes the `zurb_foundation_page_site_name` and `zurb_foundation_page_site_logo` values to false.

However, this doesn't remove the theme settings, which is maybe the goal here.

So, circling back around to this issue: I would like to lobby for the more thorough removal of these duplicate settings like in previous patches. I think that it is confusing to have these settings because they're already in core, and they don't add any particularly exciting functionality once they're toggled on.

I manually re-rolled patch #21 and did a bit more cleanup to remove documentation for these settings in the .info file, the STARTER info file, and the page template's doc-block.

I didn't remove the site-name variable from the menu.inc file because that's used in the responsive topbar. I think refactoring that is a different issue.

I've successfully applied @sim_1's patch. I agree with her, refactoring is a different issue and thank you for this great patch. :)

Ok, I submitted the patch. This is my first commit so I hope it worked correctly. Marking this as fixed.

Looks really good, thank you Sumit!! Simplifying the theme is very welcome :)

Just adding comment for anyone looking, this fix completely removes linked_logo from the page template so the variable is no longer available.
To duplicate it's functionality, add the following to your child theme's THEMENAME_preprocess_page function:

  $site_name = isset($variables['site_name']) ? $variables['site_name'] : \Drupal::config('system.site')->get('name');

  $variables['logo_img'] = '';

  $logo = theme_get_setting('logo.url');

  if (!empty($logo)) {
    $elements = [
      '#theme' => 'image',
      '#uri' => $logo,
      '#attributes' => [
        'alt' => strip_tags($site_name) . ' ' . t('logo'),
        'title' => strip_tags($site_name) . ' ' . t('Home'),
        'class' => ['logo'],
      ]
    ];

    $variables['logo_img'] = $elements;
  }

  $title = strip_tags($site_name) . ' ' . t('Home');
  $url = \Drupal\Core\Url::fromRoute('<front>');
  $url->setOption('attributes', ['title' => $title]);

  $variables['linked_logo'] = '';
  if (!empty($variables['logo_img'])) {
    $variables['linked_logo'] = Link::fromTextAndUrl($variables['logo_img'], $url)->toString();
  }

  $variables['linked_site_name'] = '';
  if (!empty($site_name)) {
    $variables['linked_site_name'] = Link::fromTextAndUrl($site_name, $url)->toString();
  }