Using overlays (notification, window etc) breaks top level scrolling on touch devices #3455

Closed
vaadin-bot opened this Issue Jan 28, 2013 · 7 comments

Comments

Projects
None yet
1 participant
@vaadin-bot
Collaborator

vaadin-bot commented Jan 28, 2013

Originally by @mstahv


After using overlays, the view can be scrolled to areas that "shouldn't exist".


public class MyVaadinUI extends UI {

    @Override
    protected void init(VaadinRequest request) {
        final CssLayout cssLayout = new CssLayout();
        cssLayout.setSizeFull();
        final CssLayout layout = new CssLayout() {
            @Override
            protected String getCss(Component c) {
                return "background:green;";
            }
        };
        layout.setSizeFull();
        layout.addComponent(cssLayout);
        setContent(layout);

        Button button = new Button("Click Me with touch device");
        button.addClickListener(new Button.ClickListener() {
            public void buttonClick(ClickEvent event) {

                Notification.show("Now close this and you can scroll in mad plazez.");
                cssLayout
                        .addComponent(new Label(
                                "Thank you for clicking, now scroll (with touch device) to area without green background, which shouldn't be possible."));
            }
        });
        cssLayout.addComponent(button);
    }

}

Repeatable at least in all webkit based touch devices.

The same issue might be the cause for weird double height screenshots from WebDriver/TestBench as they take screenshot of the whole body, not visible viewport only.

@vaadin-bot

This comment has been minimized.

Show comment Hide comment
@vaadin-bot

vaadin-bot Jan 29, 2013

Collaborator

Originally by @mstahv


Our Dashboard demo has own hack to overcome this:


.dashboard.v-app[id*="overlays"] {
	height: 0;
}

Collaborator

vaadin-bot commented Jan 29, 2013

Originally by @mstahv


Our Dashboard demo has own hack to overcome this:


.dashboard.v-app[id*="overlays"] {
	height: 0;
}

@vaadin-bot

This comment has been minimized.

Show comment Hide comment
@vaadin-bot

vaadin-bot Jan 31, 2013

Collaborator

Originally by @jdahlstrom


The overlays div gets height: 100% from the .themename.v-app rule, just like the actual app root div, and has simple static positioning so that the height of the whole page becomes twice the browser client height. On desktop browsers this isn't a problem due to overflow: hidden, but touch devices do things differently.

At the very least the overlays div should have a classname to make customizing its behaviour easier.

Optimally, it should be 100% x 100% with position: fixed to overlay it over the whole browser viewport. However, this would require manually passing all events to the underlying elements. The CSS pointer-events property could be used on browsers that support it (IE and Opera don't.)

Another option would be making it zero sized, but then relative-width overlay widgets would not work.

Collaborator

vaadin-bot commented Jan 31, 2013

Originally by @jdahlstrom


The overlays div gets height: 100% from the .themename.v-app rule, just like the actual app root div, and has simple static positioning so that the height of the whole page becomes twice the browser client height. On desktop browsers this isn't a problem due to overflow: hidden, but touch devices do things differently.

At the very least the overlays div should have a classname to make customizing its behaviour easier.

Optimally, it should be 100% x 100% with position: fixed to overlay it over the whole browser viewport. However, this would require manually passing all events to the underlying elements. The CSS pointer-events property could be used on browsers that support it (IE and Opera don't.)

Another option would be making it zero sized, but then relative-width overlay widgets would not work.

@vaadin-bot

This comment has been minimized.

Show comment Hide comment
@vaadin-bot

vaadin-bot Jan 31, 2013

Collaborator

Originally by @mstahv


Relative height components would have issues I assume?

I wonder what was the grand reason in moving all "overlays" into a separate div. They used to be directly in document body. That should probably be thought again.

A potential solution with this kind of "container element" might be to place it as first element in the body and make in normally positioned and 0 height. Actual "overlays" would then have body element as position parent.

Collaborator

vaadin-bot commented Jan 31, 2013

Originally by @mstahv


Relative height components would have issues I assume?

I wonder what was the grand reason in moving all "overlays" into a separate div. They used to be directly in document body. That should probably be thought again.

A potential solution with this kind of "container element" might be to place it as first element in the body and make in normally positioned and 0 height. Actual "overlays" would then have body element as position parent.

@vaadin-bot

This comment has been minimized.

Show comment Hide comment
@vaadin-bot

vaadin-bot Jan 31, 2013

Collaborator

Originally by @jdahlstrom


There are several reasons, the most important ones having to do with theming and event handling, especially in a non-standalone context. The original ticket is #9220.

Collaborator

vaadin-bot commented Jan 31, 2013

Originally by @jdahlstrom


There are several reasons, the most important ones having to do with theming and event handling, especially in a non-standalone context. The original ticket is #9220.

@vaadin-bot

This comment has been minimized.

Show comment Hide comment
@vaadin-bot

vaadin-bot Feb 11, 2013

Collaborator

Originally by @jdahlstrom


Added a v-overlay-container classname to the overlay container to facilitate customization. The width and height of the container can be set to zero by default without ill effects, as overlays in practice will always be position: absolute or fixed, and as long as the container has default position, its relative-sized children will base their size on the body element, not the container.

Collaborator

vaadin-bot commented Feb 11, 2013

Originally by @jdahlstrom


Added a v-overlay-container classname to the overlay container to facilitate customization. The width and height of the container can be set to zero by default without ill effects, as overlays in practice will always be position: absolute or fixed, and as long as the container has default position, its relative-sized children will base their size on the body element, not the container.

@vaadin-bot

This comment has been minimized.

Show comment Hide comment
@vaadin-bot

vaadin-bot Feb 12, 2013

Collaborator

Originally by @jdahlstrom


Reviewed by John Ahlroos.

Collaborator

vaadin-bot commented Feb 12, 2013

Originally by @jdahlstrom


Reviewed by John Ahlroos.

@vaadin-bot vaadin-bot closed this Feb 12, 2013

@vaadin-bot

This comment has been minimized.

Show comment Hide comment
@vaadin-bot

vaadin-bot Feb 14, 2013

Collaborator

Originally by @mstahv


Thanks, verified to work fine with TouchKit nightly build. This was big step forward!

Collaborator

vaadin-bot commented Feb 14, 2013

Originally by @mstahv


Thanks, verified to work fine with TouchKit nightly build. This was big step forward!

@vaadin-bot vaadin-bot added the bug label Dec 9, 2016

@vaadin-bot vaadin-bot added this to the Vaadin 7.0.1 milestone Dec 9, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment