Add #button_type support for #type link

After this patch was created a new class get into D8, RenderTest, I guess we want to reuse it.

I"m not sure this is inline with the classy theme to move classes to the classy theme and remove them from stark. I'll send @mortendk and @davidhernandez over here to check into this.

Is there a prevision for the case of not wanting any classes on buttons? Or removing the 'button' class?

ooooh these prerendered classes are a pita to work with later on & one of the things that have haunted themers down when they wanted to add in other frameworks, that lets say used the .button for something specific (and you dont wanna maintain your own version of that framework offcourse)

Heres an idea - what if we prefix that kinda stuff with, so its clear where it comes from, and dont end up taking common classnames (yes i know im anal about this)

1. +++ b/core/includes/common.inc
   @@ -3105,6 +3106,17 @@ function drupal_pre_render_link($element) {
   +    $element['#options']['attributes']['class'][] = 'button';
   

   drupal-button ?

2. +++ b/core/includes/common.inc
   @@ -3105,6 +3106,17 @@ function drupal_pre_render_link($element) {
   +      $element['#options']['attributes']['class'][] = 'button--' . $element['#button_type'];
   

   drupal-button--

By at least using some kind of name spacing we know where it comes from & the chance of it colliding with an exisiting css framework is small.

What i would Looooooooove to see was instead button.html.twig and used addClass(' button--mucho-danger') instead, that would make it clear for the themer where to fix this class thingie

Yeah, I was going to say the exact same thing as Morten. Not the class prefix part ;) but the template part. We are trying to eliminate, as much as possible, classes being generated, so I don't think we want to add more. The problem is how we are styling/creating buttons in core. It would be much nicer if we had a button template, and pass a variable of the button type to it. Then themers could deal with it as they want, including changing how the markup works.

I was wrong about RenderTest, it's not a base test as the one added here.

Here an updated patch, based on what sun proposed, creates a twig template for links, which is needed in order to add classes on the template.

I modified the implementation to follow several drupal changes like link theme variables.
Also I'm using it on the LinkGenerator class.

Notes:

• I'm not sure how it will impact overall performance.
• I would not expect this to past all tests.
• This can be divided in different patches if needed.
• Notice original patch is providing a new abstract test base class, which seems to be useful, but maybe also worth to mention it in the commit.

@marvil yeah for templates ! - could we call it button-link.html.twig in that its clear for the themer that dont know anything what the template might is about.

What about just link--button-danger.html.twig and just add to the template suggestions of the existing template?

A new patch, here I'm trying to use theme suggestions to pick either link.html.twig or link--button.html.twig depending on the use of the new '#button_type' property, BTW since logic is now in php we can again differentiate between isset() and empty() cases, which seems to be not possible in twig.

In any case, I'm not sure if I'm using theme suggestions correctly, since they seem to be picked up, but the template in use is always link.html.twig, any suggestion is welcome.

I forgot the interdiff

KernelTestBase already have a render(), reuse it, question about how to use twig templates is still open.

Actually use \Drupal\Tests\Core\Render\RendererTestBase

1. +++ b/core/lib/Drupal/Core/Utility/LinkGenerator.php
   @@ -137,7 +137,13 @@ public function generate($text, Url $url) {
   +    return \Drupal::service('renderer')->render($link);
   

   This should be injected.

2. +++ b/core/modules/system/src/Tests/Render/LinkTest.php
   @@ -0,0 +1,53 @@
   +  public static function getInfo() {
   +    return array(
   +      'name' => '#type link',
   +      'description' => 'Tests rendering of #type link.',
   +      'group' => 'Render',
   +    );
   +  }
   

   This can be removed

3. +++ b/core/modules/system/src/Tests/Render/LinkTest.php
   @@ -0,0 +1,53 @@
   +  function testButtonType() {
   

   public function

4. +++ b/core/modules/system/src/Tests/Render/RenderTestBase.php
   @@ -0,0 +1,63 @@
   +  protected function assertRaw($string, $message = NULL) {
   

   Why is this redclaring assertRaw and friends? Please use AssertContentTrait.

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Wanted to bump 1 more time.

May still be relevant looking at it a third time, definitely needs a summary update.