Convert theme_toolbar_toggle to twig

+++ b/core/modules/toolbar/toolbar.module
@@ -47,6 +47,7 @@ function toolbar_theme($existing, $type, $theme, $path) {
     ),
+	'template' => 'link',
   );

no tabs! use whitespaces!

good work!
thanks

patch does not apply against latest merge_chx_sandbox branch

git apply --check 1779104-toolbar-toggle-3.patch
error: patch failed: core/modules/toolbar/toolbar.module:47
error: core/modules/toolbar/toolbar.module: patch does not apply

need reroll

woops
status

commited #3 into 8.x branch

Re-opened as it was not merged to front-end branch, yet and removing theme functions should probably not be done.

@jenlampton: Could you take a look at the patch and ask author to re-roll properly?

Best,

Fabian

This will be resolved soon as we move all our work over to the modules directly.

We are that far in core now :).

http://drupal.org/node/1696786#comment-6576850

And the template => 'file' seems to work fine even if another theme is used, but needs more investigation.

#15 looks good for me
but before commit we should decide what to do with "not found" *.tpl.php templates in such cases

#15 commited / push to front-end

Thanks!

this file is stil missing from the front-end branch, can you push again?

#18 what file?

toolbar-toggle.html.twig is not in the core/themes/stark/templates/toolbar directory.

I expect that this patch was committed and just not pushed?

#20
we are not using toolbar-toggle.html.twig
we are using link.html.twig here http://drupalcode.org/sandbox/pixelmord/1750250.git/blob/5d7dd04f00eff12...

status

Ah, okay. Then we'll need a temporary file that includes link.html.twig instead with a @todo and a link to the core issue #1595614: [meta] Remove all the theme functions and templates in core that simply output a link. Replace with #type 'link' render arrays

See textfield.html.twig for an example of how this is done - but the Twig template should look something like this:

{#
/**
 * @file
 * Default theme implementation to display a whatever.
 *
 * Available variables:
 * - text: The link text for the anchor tag.
 * - path: The internal path or external URL being linked to, such as
 *   "node/34" or "http://example.com/foo".
 * - attributes: Remaining html attributes for the containing element.
 *
 * @see template_preprocess
 * @see template_preprocess_toolbar_toggle
 *
 * @ingroup themeable
 */
 @todo: remove this file once http://drupal.org/node/1595614 is resolved.
#}
{% include "core/themes/stark/templates/theme.inc/link.html.twig" %}

#24 commited / pushed to front-end

Thanks!

There are still a few problems with the file in the front-end branch:

- missing available variables in docblock
- missing include (still shows anchor tag?)

comments on anchor tag (though about to be removed)
- whitespace controllers around classes (they should never be on classes)
- missing whitespace controller on left of attributes (should always have one on attribtutes)

It looks like this line was just copied from link.html.twig, so I'll update that file too. Sorry for the confusion.

Just for reference, I updated the template file in front-end to be the following:

{#
/**
 * @file
 * Default theme implementation for a toolbar drawer's visibility.
 *
 * Available variables:
 * - text: The translated link text for the anchor tag.
 * - attributes: Remaining html attributes for the containing element.
 * - attributes.class: HTML classes that can be used to style contextually
 *     through CSS.
 * - path: The internal path or external URL being linked to, such as
 *   "node/34" or "http://example.com/foo".
 * - options: Additional options, with the following elements:
 *   - options.attributes: An associative array of html attributes to apply
 *     to the anchor tag. If element 'class' is included, it must be an
 *     array; 'title' must be a string; other elements are more flexible,
 *     as they just need to work in a call to
 *     drupal_attributes($options['attributes']).
 *   - options.html (default FALSE): Whether $text is html or just plain-text.
 *     For example, to make an image tag into a link, this must be set to
 *     TRUE, or you will see the escaped html image tag. text is not
 *     sanitized if 'html' is TRUE. The calling function must ensure that
 *     text is already safe.
 *   - options.language: An optional language object. If the path being
 *     linked to is internal to the site, $options['language'] is used to
 *     determine whether the link is "active", or pointing to the current
 *     page (the language as well as the path must match). This element is
 *     also used by url().
 *   - Additional options elements used by the url() function.
 *
 * @see template_preprocess
 * @see template_preprocess_toolbar_toggle
 *
 * @ingroup themeable
 */
 @todo: clean up url() once http://drupal.org/node/1812562 is resolved.
 @todo: remove this file once http://drupal.org/node/1595614 is resolved.
#}
{% include "core/themes/stark/templates/theme.inc/link.html.twig" %}

This is unnecessary. Let's do like #1595614: [meta] Remove all the theme functions and templates in core that simply output a link. Replace with #type 'link' render arrays and use theme('link') instead. ... Patch coming up.

This is what I'm talking about (attached).

Depends on #1 in #1825820: theme('link').

Last patch was missing an important line that switched the toolbar-active class on and off.

To the point:

1. Apply this from #1825820: theme('link')
2. Apply this from #1825828: theme('toolbar')
3. That's how I think toolbar should work.

Sorry for all the posts. I'm trying to keep patches on their own topics.

#31 please better use
[#topicID-commentNUM] as #1779104-31: Convert theme_toolbar_toggle to twig for Your comment to point to right place(issue node + comment) of a patch

Will do, podarok.

#33 what is here to review?
i`d lost myself :(

podarok: see #1825828-3: theme('toolbar') for full patch for toolbar.

bad comment
sorry

This patch removes theme('toolbar_toggle') and replaces it in theme('toolbar') with an implementation of theme('link').

It fixes the toolbar toggle issue I noticed (Hide shortcuts and Edit shortcuts links).

Before:

After:

Patch would be helpful.

Hi all, I just wanted to mention that the toolbar theme issues are being addressed in the responsive toolbar patch. I just switched from tpl to twig in the latest patch.

#1137920-282: Fix toolbar on small screen sizes and redesign toolbar for desktop

I would suggest closing this issue as a duplicate or at least postponing it. I'd like to avoid you all spending time on unnecessary work here.

Admin Toolbar overlay isn't showing in stark.

Accidentally reopened this one... meant to post the previous comment on #1825828: theme('toolbar')