Problem/Motivation

With the addition of the style discovery, we can style a paragraphs adding some pre defined styles. We are adding a Link paragraph type (#2852384: New Type: Link) and it would be used to have a button style.

Proposed resolution

Add a core based button style that can be used on the Link fundamental paragraph type.

Remaining tasks

  1. Add more styles, see last paragraph in #18
  2. Update the link paragraph type
  3. Add demo content
  4. Add tests
  5. Address css revie

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yongt9412 created an issue. See original summary.

johnchque’s picture

This is the button style. Looks like:

With that I realized that maybe using a style is not the best approach IMHO, we better add a plugin that adds the "button" class so it follow any theme that is used. Anyway, this is copied from core, seven theme.

johnchque’s picture

This is the button style. Looks like:

With that I realized that maybe using a style is not the best approach IMHO, we better add a plugin that adds the "button" class so it follow any theme that is used. Anyway, this is copied from core, seven theme.

miro_dietiker’s picture

Primsi’s picture

Tested this, all looks fine. Asked @pivica to take a look.

Primsi’s picture

Status: Needs review » Needs work

Hm,.. or maybe you're right. Having a behavior might be better. Because in addition to the thing you already mentioned, currently we only allow one style per paragraph, so a style that would like to have both style and it's links turned to buttons, would have to contain the css for the button too.

pivica’s picture

Hm,.. or maybe you're right. Having a behavior might be better.

This is really a link element with a button style. We already discussed a bit this problematic on https://www.drupal.org/node/2852384#comment-11941829 and couple of comments bellow that one. I think by default we should not provide any styling to this link element, but move this to plugin style. Most of the times people will want a button look, but also some times they will want link lool. Also its common to have couple of button style variations.

Here is a first CSS review:

  1. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  box-sizing: border-box;
    

    This days most of the css frameworks are defining border-box sizing model for all HTML elements by default.
    Maybe we should do something similar for all of our paragraph types by default?

  2. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  position: relative;  /* 1 */
    

    Why do we define position relative here?

  3. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  line-height: normal;  /* 2 */
    

    Btw, what is with this /* 2 */ and other comments?

  4. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  -webkit-appearance: none;  /* 3 */
    +  -moz-appearance: none;  /* 3 */
    

    Not sure about this two, why do we need them?

    Accoding to https://developer.mozilla.org/en/docs/Web/CSS/-moz-appearance none is already default value for apperance.

    Also core is using -webkit-appearance: button; for button input elements. Does it make sense that we use the same here?

  5. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  background-image: -webkit-linear-gradient(top, #f6f6f3, #e7e7df);
    

    http://caniuse.com/#feat=css-gradients - coverage of linear-gradient is pretty good and there is no need for -webkit prefixed version.

  6. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  font-size: 14px;
    +  font-size: 0.875rem;  /* 5 */
    

    Any special reason for two font-size properties?

  7. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  -webkit-transition: all 0.1s;
    

    http://caniuse.com/#search=transition, same thing pretty good coverage we can drop -webkit prefix.

  8. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  -webkit-font-smoothing: antialiased;  /* 6 */
    

    This is non standard css rule and we should not use it i think https://developer.mozilla.org/en-US/docs/Web/CSS/font-smooth. Also this kind of stuff should really be done on theme level and not module level.

  9. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  background-image: -webkit-linear-gradient(top, #fcfcfa, #e9e9dd);
    

    We can drop - webkit prefix.

  10. +++ b/modules/paragraphs_collection_demo/css/button-link.css
    @@ -0,0 +1,53 @@
    +  background-image: -webkit-linear-gradient(top, #f6f6f3, #e7e7df);
    ...
    +  -webkit-transition: none;
    

    We can drop prefixes.

miro_dietiker’s picture

I don't want to explain to end users to use a "class" plugin and manually type "button" in the class text field.

I believe a managed style could be specific with which link fields are output as buttons. For sure we should not unconditionally force all links in the paragraph (container) switched to button style.

It's possible that the paragraph type / style a user picks defines that.
As a start, we could define a separate style group "Link" and only allow the use of this style on the link paragraph.

pivica’s picture

> I don't want to explain to end users to use a "class" plugin and manually type "button" in the class text field.

I was thinking more that we add this to plugin style actually. Is it possible to attach default plugin style when attaching plugin to some paragraph type so user does not need to select style each time it is adding new paragraph?

Primsi’s picture

Yea, having the button formatting applied to every link is not so great I think. That's why I thought that the suggested workflow would be:
1. add container pt (paragraph type) with style
2. add a link pt with the behavior "Make button" turned on.

If we make this a behavior we could even require the pt to have a url type field. Not sure if we want to go there, but that's an option too.

Ginovski’s picture

Title: Provide a button style » Provide a button style group
Issue summary: View changes

Another approach to the issue

drobnjak’s picture

Assigned: Unassigned » drobnjak

I will work on this.

Ginovski’s picture

Title: Provide a button style group » Provide a button style
Issue summary: View changes

Reverting issue for more general approach.

drobnjak’s picture

Assigned: drobnjak » Unassigned
pivica’s picture

1. add container pt (paragraph type) with style
2. add a link pt with the behavior "Make button" turned on.

@Primsi didn't quite get it why do we need a container pt with style for this?

miro_dietiker’s picture

So yeah, by default links should be links.

And then we offer to convert them to buttons of different emphasis:
- ghost (just an outline border, no background)
- secondary action
- primary action

The label "Link" needs to go away.

Primsi’s picture

@pivica Not sure if I remember exactly, but the main point was that perhaps it's better to have a behavior instead of a paragraph type with a specific styling.

@miro_dietiker How do we offer this choice? By having a behavior that offers a selection of button styles?

pivica’s picture

> @pivica Not sure if I remember exactly, but the main point was that perhaps it's better to have a behavior instead of a paragraph type with a specific styling.

Yes sure we have a style behavior for button which is visually converting link type to button look. I asked why would we apply that kind of a style to a container, this kind of style should work on a lint type level right?

> @miro_dietiker How do we offer this choice? By having a behavior that offers a selection of button styles?

I guess we have a style group Button that can be applied to link par type. Some styles could be Primary, Secondary, Warning, etc.

Primsi’s picture

> I asked why would we apply that kind of a style to a container

We were discussing what would happen if the style for buttons was applied to a paragraph of type container. Then all links inside of it would be turned into buttons. Which might not always be what we want.

But I guess if we let the sitebuilder have the responsibility to not do that and apply this style only to link type paragraphs (or others if the desired effect is to turn every a into buttons), that's also fine.

And yes, it adds less complexity, especially if we want to offer different kind of buttons.

So what this patch still need is:

  1. Add more styles
  2. Update the link paragraph type
  3. Add demo content
  4. Add tests
  5. Address css review
Primsi’s picture

Issue summary: View changes
pivica’s picture

> We were discussing what would happen if the style for buttons was applied to a paragraph of type container. Then all links inside of it would be turned into buttons.

Okey but then CSS rules would be slightly different for button style that is applied to container and for button style that is applied to link - so we will have two style types if we want to cover this or we have a style rule with selectors that can cover both cases?

Primsi’s picture

From my point of view I am fine if we don't cover the style applied to non-link paragraph types. If we see it causes troubles we can fix that later IMHO.