vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
617 stars 167 forks source link

style with invalid value not set to DOM #12182

Open mvysny opened 2 years ago

mvysny commented 2 years ago

Description of the bug / feature

Consider the following code:

        final FlexLayout flexLayout = new FlexLayout();
        flexLayout.getStyle().set("grid-template-columns", "fit-content 1fr");

I'm setting an invalid value to the grid-template-columns style. Yes, it is my bad and I should have known better. However, the grid-template-columns style is completely lacking from the browser's DOM for no apparent reason! I've just wasted an hour debugging Flow code, trying to figure out why the grid-template-columns style would not be applied client-side.

Minimal reproducible example

Paste this code into skeleton starter:

package org.vaadin.example;

import com.vaadin.flow.component.orderedlayout.FlexLayout;
import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.router.Route;

@Route("")
public class MainView extends VerticalLayout {

    public MainView() {
        final FlexLayout flexLayout = new FlexLayout();
        flexLayout.getStyle().set("grid-template-columns", "fit-content 1fr");
        add(flexLayout);
    }
}

Expected behavior

The flexlayout div should have the grid-template-columns style set. A browser would then give me a warning that I'm setting something dumb there.

Actual behavior

The flexlayout div is completely and silently lacking the grid-template-columns style, giving no indication as to what's wrong.

Versions:

- Vaadin / Flow version: 21.0.2/8.0.2
- Java version: 11
- OS version: Ubuntu 21.10 x86-64
- Browser version (if applicable): Firefox 93.0
- Application Server (if applicable): Tomcat, but doesn't really matter
Artur- commented 2 years ago

You can try this in a Chrome dev tools

$0.style.gridTemplateColumns = 'fit-content 1fr'
console.log($0.style.gridTemplateColumns);

It does not set the value nor does it print any errors

mvysny commented 2 years ago

Oh, so it is a bug/feature in the browser rather than in Flow. I see, thank you.

Perhaps there could be a way for Flow to somehow validate the values of CSS variables and then log something into the JS console (perhaps there is a JavaScript library for that)?

knoobie commented 2 years ago

I don't think that yet another library is worth it.. does the client side know about production mode / development mode?

I think it would easier to check on development mode something like $.style.XYZ == 'myValue' after the style was applied? If true; all good - if it fails: console.log?

mvysny commented 2 years ago

Awesome idea :+1:

Artur- commented 2 years ago

I think that would give too many false positives as the value is normalized when set , e.g. with this simplistic example

> $0.style.border="1px solid black;"
> $0.style.border === "1px solid black;"
< false
mvysny commented 2 years ago

True. Perhaps better would be to check whether the new value simply reverted to an empty string - if yes then the value is invalid:

Screenshot from 2021-10-27 15-03-40

Actually, in my case if the value is invalid, the browser will simply not apply the value and will retain the previous value. Therefore, the test would be not for an empty string, but whether the value actually changed.