search

vaadin / vaadin-button-flow

Vaadin Flow Java API for vaadin/vaadin-button Web Component
https://vaadin.com/components/vaadin-button
Other
0 stars 7 forks source link

getElement().setAttribute('role', 'custom-role') isn't working upon attachment #172

Closed tarekoraby closed 4 years ago

tarekoraby commented 4 years ago

getElement().setAttribute() isn't working if called upon the Button's attachment (it seems to work if called later).

To reproduce:

Button button = new Button("button");
button.getElement().setAttribute("myAttribute", "value");

Expected: 'myAttribute' is added to the button, however, no such attribute is added upon inspection.

Tried in a Vaadin 14.3.7 + Spring boot project

tarekoraby commented 4 years ago

The workaround via a JavaScript call works though:

button.getElement().executeJs("this.setAttribute('role', 'custom-role')");

alvarezguille commented 4 years ago

Couldn't reproduce this with v14 branch of skeleton-starter-flow-spring Added

        textField.getElement().setAttribute("myAttribute", "value");
        button.getElement().setAttribute("myAttribute", "value");

To MainView.java and attribute is correctly added to both elements.

alvarezguille commented 4 years ago

Can't be reproduced, closing

DavidKoenig commented 4 years ago

It is working for any attribute, except "role".

@tarekoraby is referencing this in

The workaround via a JavaScript call works though:

button.getElement().executeJs("this.setAttribute('role', 'custom-role')");

Maybe the heading of the issue could be more specified to the role attribute.

tarekoraby commented 4 years ago

It is working for any attribute, except "role".

@DavidKoenig that's true. I had a discussion about this with @alvarezguille earlier today and we wondering if you could please elaborate a bit on your use case: Why do you need to override the 'role' attribute?

Please also note, that getElement().setAttribute('role', 'custom-role') should only affect the <vaadin-button> element, not the <button> one embedded within. So, if the aim is to alter the latter, then something like this JS call would be necessary perhaps:

button.getElement().executeJs("this.shadowRoot.getElementById('button').setAttribute('role','custom-role')");

web-padawan commented 4 years ago

I think we should disallow using custom role attribute for accessibility reasons.

In future we might change this and remove role="button" from the host element, while also removing role="presentation" from the native button, as it seems to causes problems in some cases.

See https://github.com/vaadin/vaadin-button/issues/96#issuecomment-579758891

DavidKoenig commented 4 years ago

@tarekoraby We wanted to change the attribute on the <vaadin-button> and inner <button> for accessibility reasons, but our assumption was wrong and the Vaadin implementation is correct. Nevertheless a better documentation about attributes that you can't change on certain components, like the button or the items in a MenuBar, would be nice. Otherwise you always wonder why you can't change attributes on one component via element api but not on the other.