feat: mark full size components with data attribute #20929
Conversation
flow-server/src/main/java/com/vaadin/flow/component/HasSize.java
Outdated
Show resolved
Hide resolved
|
@mshabarov I've dropped the |
| @@ -47,6 +47,7 @@ public interface HasSize extends HasElement { | |||
| */ | |||
| default void setWidth(String width) { | |||
| getElement().getStyle().setWidth(width); | |||
| getElement().removeAttribute("data-width-full"); | |||
There was a problem hiding this comment.
This would leave a small gap which is when you call getElement().getStyle().setWidth() instead (add-ons may use this), you'll not get this attribute removed.
I was thinking of adding removeAttribute to the getElement().getStyle().setWidth(), but there is no direct access to the Element API. However, we can check if this is an Element, case and call this method.
What do you think, does it worth it or we can make an assumption that component's method would be sufficient for most of the usages?
There was a problem hiding this comment.
I did take a look at Style and the probably relevant BasicElementStyle subclass, but am not sure how get a reference to the element from there.
In general I would assume that:
- Users would usually use
HasSizeconsistently - Probably not switch between full size and custom size that often
For now I would probably go with what we have. If this turns out to be a problem we have another option of changing the CSS selector in the web components to check for both, the data attribute and style="width: 100%" / style="height: 100%".
| @@ -194,6 +195,7 @@ default Optional<Unit> getWidthUnit() { | |||
| */ | |||
| default void setHeight(String height) { | |||
| getElement().getStyle().setHeight(height); | |||
| getElement().removeAttribute("data-height-full"); | |||
There was a problem hiding this comment.
Could we place these constants to Constants ?
There was a problem hiding this comment.
I think you have a better idea what kind of constants that class should contain. I think it wasn't too bad with just a few duplicates and covering those with unit tests. But I've extracted the attributes into constants for now.
| getElement().setAttribute("data-width-full", true); | ||
| getElement().setAttribute("data-height-full", true); |
There was a problem hiding this comment.
Would it better to just call
setWidthFull();
setHeightFull();
where these attributes are already added?
There was a problem hiding this comment.
Makes sense, done.
|
|
This ticket/PR has been released with Vaadin 24.7.0.alpha7 and is also targeting the upcoming stable 24.7.0 version. |
Description
Adds data attributes to components that have been configured to use the full size in a layout, by either calling
setWidthFull,setHeightFullorsetSizeFull. Vaadin components such asHorizontalLayoutandVerticalLayoutwill include additional styles targeting children with these data attributes, to effectively make these children take up the remaining space in a layout, rather than the full size.For now, the respective styles will only be added by the Vaadin components if the
com.vaadin.experimental.layoutImprovementsfeature flag is enabled (see vaadin/web-components#8552).Part of vaadin/flow-components#6998
Type of change