Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lumo] Styling links as buttons #1803

Open
jouni opened this issue Jan 18, 2018 · 29 comments
Open

[lumo] Styling links as buttons #1803

jouni opened this issue Jan 18, 2018 · 29 comments
Labels
enhancement New feature or request theme
Projects

Comments

@jouni
Copy link
Member

jouni commented Jan 18, 2018

From @heruan on November 16, 2017 18:7

I guess this is borderline, but it would be very useful to style links as buttons, e.g. to let the use CMD + click and open a new tab.

How much the templating system can help on applying the valo-button theme module to an anchor element, with all variants, without the need to maintain a duplicate style?

Copied from original issue: vaadin/vaadin-valo-styles#34

@jouni
Copy link
Member Author

jouni commented Jan 18, 2018

Yeah, would be useful in some cases. I actually wanted to do this in the theme documentation pages, and ended up prototyping this a bit.

It’s possible but requires pretty much 2x selectors in the valo-button style module, so I’m wondering if that’s an acceptable tradeoff?

The way you would use this:

<link rel="import" href="../vaadin-button.html">

<custom-style>
  <style include="valo-button">
  </style>
</custom-style>

<a href="..." theme="button primary large">Button link</a>

Examples of the duplication of selectors used in the style module:

:host,
[theme~="button"]


:host(:hover:not([theme*="tertiary"]):not([active])),
[theme~="button"]:hover:not([theme*="tertiary"]):not([active])


:host::before,
:host::after,
[theme~="button"]::before,
[theme~="button"]::after


:host([active]:not([theme~="primary"]):not([theme*="tertiary"])),
[theme~="button"]:active:not([theme~="primary"]):not([theme*="tertiary"])

@jouni
Copy link
Member Author

jouni commented Jan 18, 2018

From @heruan on November 24, 2017 17:45

I didn't understand entirely the duplication of selectors, although it seems acceptable to me. The use case is intuitive and clean! 👍

@jouni
Copy link
Member Author

jouni commented Jan 18, 2018

Basically, wherever there’s a :host selector, we also need to duplicate it and replace :host with [theme~="button"], becuase :host won’t match anything outside a custom element.

The [active] state needs to be changed to :active as well.

@jouni
Copy link
Member Author

jouni commented Jan 18, 2018

From @heruan on January 2, 2018 17:14

@jouni Now that the button theme is moved to the button repository, where's the best location to include this (if you're okay to proceed with it, of course!)?

@jouni
Copy link
Member Author

jouni commented Jan 18, 2018

This repo would still be the logical place for the styles. vaadin-button would then just include the style modules from this package.

I still don’t exactly know how to pull this off nicely. My previous suggestions doesn’t really work, as the :host selectors can end up matching unwanted elements (when you import&include the style module in another custom element).

The only other option I now see is to have some build script that generates a vaadin-button specific style module from a more generic module. Since there’s one other case that needs a build script (https://github.com/vaadin/vaadin-valo-styles/blob/master/color.html#L190), it might be okay to introduce a build step for this package.

@heruan
Copy link
Member

heruan commented Mar 30, 2018

Maybe the complementary is easier? See vaadin/vaadin-button#79

Both would be useful since the two use cases can be very common (navigate when clicking a button and trigger something when clicking a link).

@heruan
Copy link
Member

heruan commented Sep 5, 2018

@jouni Still without any clue on how to do this nicely? I'd really make use of this, for example to trigger a download in Flow you should use an anchor, i.e.

new Anchor("Download me!", streamResource);

but placing this along buttons in a toolbar isn't quite pleasant 👊👁

@jouni
Copy link
Member Author

jouni commented Sep 6, 2018

No bright ideas yet.

Have you tried: <vaadin-button><a>...</a></vaadin-button> (or <a><vaadin-button>...</vaadin-button></a>)? I bet it has some drawbacks, like having a double tab target (you’d need to set tabindex=-1 for the link), but it might be workable somehow.


I tried one approach in my j-elements research project, how this kind of ”portable CSS” could be used in web components.

Basically, I author the component CSS according to BEM (or close to at least), and the rewrite the selectors when using it inside a shadow root, using the bemToShadow function.

For example, here’s the styles for <j-avatar>: https://github.com/jouni/j-elements/blob/master/src/styles/avatar-style.js

And here’s how it’s used in the avatar: bemToShadow(style, '.j-avatar')

  • style is the string from avatar-style.js
  • .j-avatar is the “block” selector that should be replaced with :host

@AdrienC
Copy link
Contributor

AdrienC commented Sep 11, 2018

Hi,
I tried <a><vaadin-button>...</vaadin-button></a> to make a RouterLink look like a button, but my whole page was reloaded, so I lost the benefits of my RouterLink.
I tried to style the vaadin-button with 'pointer-events: none' but the button did not have its :hover style and the blur when clicked.
So I disabled pointer-events on the inner <button> of the webcomponent, and it works:

RouterLink link = new RouterLink("", MainView.class);
Button button = new Button("Home");
button.getElement().setAttribute("tabindex", "-1");
link.add(button);
button.addAttachListener(event -> event.getUI().getPage().executeJavaScript("$0.shadowRoot.lastElementChild.style.pointerEvents = 'none';", button.getElement()));

@probert94
Copy link

probert94 commented Feb 26, 2019

I just wanted to share my current solution:
I created a custom Component like this:

public class CustomAnchor extends Composite<Anchor> implements HasTheme, HasStyle {
    /** Custom-Attribut, used to identify the `vaadin-button` as child of `CustomAnchor` */
    private static final String ATTR_ANCHOR        = "anchor";

    private Button btn;

    public CustomAnchor() {
        super();
        btn = new Button();
        btn.setSizeFull();
        btn.getElement().setAttribute(ATTR_ANCHOR, "");
    }
    public CustomAnchor(String href, String text) {
        this();
        setHref(href);
        setText(text);
    }
    // ===============================
    // Other constructors, matching the ones in Anchor
    // ===============================
    protected Anchor initContent() {
        Anchor a = super.initContent();
        a.add(btn);
        return a;
    }
    public void setHref(String href) {
        getContent().setHref(href);
    }
    public void setText(String text) {
        btn.setText(text);
    }

    // Override methods from HasStyle and HasTheme
    // Only methods calling "getElement" have to be overridden

    public void setThemeName(String themeName) {
        btn.setThemeName(themeName);
    }
    // =============================
    // Other methods form HasStyle and HasTheme
    // =============================

    // Support router-link
    public void setRouterLink(boolean value) {
        if (value)
            getElement().setAttribute(ApplicationConstants.ROUTER_LINK_ATTRIBUTE, "");
        else
            getElement().removeAttribute(ApplicationConstants.ROUTER_LINK_ATTRIBUTE);
    }
}

Basically I create a Composite<Anchor> with a Button. In initContent this Button is added to the Anchor as a child.
I implemented setHref, which simply calls setHref for the Anchor.
The method setText instead calls setText for the Button.
To be able to style and theme the CustomAnchor the same way you would style and theme a Button, I had to implement HasStyle and HasTheme and override all of the methods which call getElement.
This methods call the respective methods from Button.

To support router-link, I created a method setRouterLink(boolean) which adds / removes the router-link attribute.
As mentioned in the comment by @AdrienC the router-link is not working by default and the whole page will be reloaded.
Based on his workaround, which disables the pointer-event for the inner <button>, I created a dom-module which does exactly that:

<dom-module id="custom-anchor" theme-for="vaadin-button">
    <template>
        <style>
            :host([anchor]) #button {
                pointer-events: none;
            }
        </style>
    </template>
</dom-module>

Sidenote:
this solution is not lumo-specific and should work with all other themes too. I tested it with Lumo and Material

@probert94
Copy link

probert94 commented Feb 28, 2019

I just noticed an issue with the solution I posted above:
If I click the label inside the button, the routing works as expected, but if I click outside the label, near the border of the button, the page will be reloaded.
Adding pointer-events: none to the vaadin-button solves the issue, but then you won't get the hover effects.

EDIT:
To solve this issue, the content must be the same size as the vaadin-button.
This way, the only the nested button would get the events and as you set pointer-events: none, it won't handle them.
The updated custom style looks like this:

<dom-module id="custom-button" theme-for="vaadin-button">
    <template>
        <style>
            :host([anchor]) {
                /* The margin of the vaadin-button causes the a-Tag to be slightly larger then the button. */
                margin: 0px;
            }
            :host([anchor]) #button {
                pointer-events: none;
            }
            /** The "label"-Part of the button needs to use the full size. */
            :host([anchor]) [part="label"] {
                width: 100%;
                height: 100%;
                padding: 0px;
            }
            /**
             * The horizontal-layout, which is the container of the label-content needs to use the full size.
             * Use align-items: center to center the content vertically.
             */
            :host([anchor]) [part="label"] ::slotted(vaadin-horizontal-layout) {
                width: 100%;
                height: 100%;
                align-items: center;
            }
        </style>
    </template>
</dom-module>

@heruan
Copy link
Member

heruan commented Feb 28, 2019

Thank you @Springrbua for sharing this! I'd like to hear the team's take on it, as it might be worth a PR. Or maybe there's already something on the works with the BEM strategy explained by @jouni?

Sure thing is: styling a link as a button should be easy, as in Bootstrap's <button class="btn"> and <a class="btn">.

@probert94
Copy link

@heruan while my solution works pretty good, I consider it a workaround and I don't think it should become the official solution.
@jouni a possible solution to overcome the duplicate selectors would be to create a custom tag like the vaadin-button does.
The vaadin-button also seems to somehow wrap the native button, the vaadin-anchor could do something similar.
Not sure if thats possible though.

@jouni
Copy link
Member Author

jouni commented Mar 1, 2019

One additional point to consider when nesting a button inside an anchor: there should be only one focusable element when navigating with the keyboard. So the nested button should probably have tabindex=-1. Also, I’m not sure how screen readers interpret this combination.

@jouni a possible solution to overcome the duplicate selectors would be to create a custom tag like the vaadin-button does.

Yes, that’s one solution, true. Pretty much exactly the same solution as with <vaadin-button>, that we want easy-to-style buttons/links. Though, my distaste for this approach has been growing lately. I think I would rather use the native elements directly, and have some additional features on top of them (like shimming the :focus-visible pseudo-class) and then some way to include a stylesheet for them in any style scopes that want it.

@probert94
Copy link

One additional point to consider when nesting a button inside an anchor: there should be only one focusable element when navigating with the keyboard. So the nested button should probably have tabindex=0. Also, I’m not sure how screen readers interpret this combination.

Thanks @jouni for the reminder, I did not notice this.
Adding btn.getElement().setAttribute("tabindex", "-1"); fixed this.

@kdeda
Copy link

kdeda commented Jul 3, 2019

CustomAnchor works perfectly. Thanks.

@bihu1
Copy link

bihu1 commented Apr 2, 2021

Based on the previous answer I think the simplest way to solve problem is creating own css class:

 .reload-disable:active {
	pointer-events: none;
}
.reload-disable {
	z-index: -1;
}

And after that:

        Button button = new Button();
	button.setClassName("reload-disable");
	RouterLink routerLink = new RouterLink();
	routerLink.add(button);

In this way hover works perfect.

@vaadin-bot vaadin-bot transferred this issue from vaadin/vaadin-lumo-styles May 19, 2021
@vaadin-bot vaadin-bot added enhancement New feature or request theme labels May 19, 2021
@web-padawan web-padawan changed the title Styling links as buttons [lumo] Styling links as buttons May 20, 2021
@knoobie
Copy link
Contributor

knoobie commented Sep 2, 2021

@rolfsmeds Could this be considered an a11y enhancement or even fix (looking at the examples from above)? A lot of developer are forced to use a wrong element (button) because styling the link to look exactly like a button is hard and a lot of businesses require it to look exactly like the other actions in a row of actions.

@rolfsmeds
Copy link
Contributor

I think it would be a bit of a stretch to call it an a11y issue. Some would even consider the opposite an a11y issue, with the argument that elements that look like buttons are expected to perform actions rather than navigation, so a link with the appearance of a button is presenting the wrong affordance. But if we disregard that as a bit too "purist", I do think it would be a worthwhile feature regardless of how we label it.

@lkraav
Copy link
Contributor

lkraav commented Oct 15, 2021

I notice @vaadin/button template has been updated to become <button>-less, re https://twitter.com/serhiikulykov/status/1447621616507174915

<div class="vaadin-button-container">
<span part="prefix">
<slot name="prefix"></slot>
</span>
<span part="label">
<slot></slot>
</span>
<span part="suffix">
<slot name="suffix"></slot>
</span>
</div>

Looking at https://a11y-vaadin-playground.netlify.app it seems like we could now put <a> element into label slot, and this issue is effectively fixed with it?

image

@knoobie
Copy link
Contributor

knoobie commented Oct 15, 2021

The <vaadin-button> is technically speaking still a button because of role=button - adding an <a> inside, is as wrong as before from an accessibility standpoint.

@lkraav
Copy link
Contributor

lkraav commented Oct 15, 2021

The <vaadin-button> is technically speaking still a button because of role=button - adding an <a> inside, is as wrong as before from an accessibility standpoint.

It seems we could simply override link button role when using it as such

// By default, if the user hasn't provided a custom role,
// the role attribute is set to "button".
if (!this.hasAttribute('role')) {
this.setAttribute('role', 'button');
}

Example

<vaadin-button role="link">
  <a href="https://github.com/vaadin/web-components/tree/master/packages/button/">
</vaadin-button>

@web-padawan
Copy link
Member

Yes it's possible to override role attribute, but it's not something we can recommend for styling links. But you can try it.

@lkraav
Copy link
Contributor

lkraav commented Oct 15, 2021

Yes it's possible to override role attribute, but it's not something we can recommend for styling links. But you can try it.

Can you also post some thoughts on the rationale?

I did the technical analysis at vaadin/vaadin-button#142 (comment), and you can't combine native <a> with <button> element in either direction.

But <vaadin-button> is not a native element, and there seems to be nothing technically or semantically wrong with nesting an anchor in it when role="link" is used.

Alternatively: is a re-usable vaadin-button stylesheet somewhere in future cards?

@web-padawan
Copy link
Member

there seems to be nothing technically or semantically wrong with nesting an anchor in it when role="link" is used.

I would say it's worth trying 👍

Alternatively: is a re-usable vaadin-button stylesheet somewhere in future cards?

I don't think so. @jouni WDYT about this?

@jouni
Copy link
Member Author

jouni commented Oct 15, 2021

I’m still hoping for that future as well, where we can have component style sheets in plain .css files. But I think that still waits for "CSS module scripts" to happen, so that we wouldn't need a build step to include the styles in the web components (or duplicate the styles in two files).

@anoblet
Copy link

anoblet commented Jan 17, 2023

We're running into this issue again when trying to track links though have them styled as a button

@lkraav
Copy link
Contributor

lkraav commented Jan 18, 2023

We're running into this issue again when trying to track links though have them styled as a button

In the meanwhile I did the manual work of copying vaadin/button shadow styles for a base library we're using conversionxl/pure@35e0ec1#diff-0c917aace88bc9103d58d9aae26950008e38fd3877e71e20c11c5d732ef5c4c6

Main problem will be keeping things in sync with new vaadin/button changes, I have no immediate automation ideas here. AFAICS just need to eyeball vaadin/button template diffs, and figure out what to carry over, or not.

@probert94
Copy link

I think I would rather use the native elements directly

Angular Material does something similar. You basically create a native button (or anchor) and add the mat-button directive to it. This directive adds the neccessary styles and features, and the same code can be used for both, the button and the anchor.

So you are probably right, it would be the better solution to remove the wrapper for button then to create another wrapper for the anchor tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request theme
Projects
No open projects
vaadin-core
  
📬  Inbox
Development

No branches or pull requests