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

Component in grid header is not displayed correctly #12622

Open
Tymur-Lohvynenko-Epicor opened this issue May 23, 2024 · 5 comments
Open

Component in grid header is not displayed correctly #12622

Tymur-Lohvynenko-Epicor opened this issue May 23, 2024 · 5 comments

Comments

@Tymur-Lohvynenko-Epicor

Vaadin Framework version - 8.24.0
Google Chrome Version 125.0.6422.77 (Official Build) (64-bit)
Windows 11 Enterprise Version 23H2

Component in grid header is not displayed correctly if you set it from background thread.
image

To reproduce bug you should change header from background thread:

    protected void init(VaadinRequest vaadinRequest) {
        grid.getDefaultHeaderRow().getCell(columnId)
                .setComponent(new HorizontalLayout(new Label("Status")));
        
        CompletableFuture.supplyAsync(() -> {
                    try {
                        Thread.sleep(2000);
                    } catch (InterruptedException ignored) {}
                    return "";
                })
                .whenComplete((s, thr) -> {
                    UI ui = getUI();
                    ui.access(() -> {
                        grid.getDefaultHeaderRow().getCell(columnId)
                                .setComponent(createLayoutForStatusColumnInHeader());
                        ui.push();
                    });
                });
    }
                
    private HorizontalLayout createLayoutForStatusColumnInHeader() {
        Button button = new Button();
        button.addStyleNames(
                ValoTheme.BUTTON_SMALL,
                ValoTheme.BUTTON_ICON_ONLY,
                ValoTheme.BUTTON_BORDERLESS
        );
        button.setIcon(VaadinIcons.REFRESH);
        button.addClickListener(clickEvent -> Notification.show("refresh!"));

        HorizontalLayout horizontalLayout = new HorizontalLayout();
        Label label = new Label("Status");
        horizontalLayout.addComponentsAndExpand(label);
        horizontalLayout.setComponentAlignment(label, Alignment.MIDDLE_CENTER);
        horizontalLayout.addComponent(button);
        horizontalLayout.setComponentAlignment(button, Alignment.MIDDLE_CENTER);
        return horizontalLayout;
    }

Push mode is @Push(value = PushMode.MANUAL, transport = Transport.LONG_POLLING)

Also you can find full code in repo-vaadin8-component-in-grid-header-bug.

@thevaadinman
Copy link
Contributor

This might be related to a situation similar to the one in #12608

@Ansku
Copy link
Member

Ansku commented May 30, 2024

Does adding ui.runAfterRoundTrip(() -> grid.recalculateColumnWidths()); before the ui.push(); help?

@Tymur-Lohvynenko-Epicor
Copy link
Author

Hello @Ansku, thank you, it helps. Can you explain what is round trip and why we should use runAfterRoundTrip but not just recalculateColumnWidths?

@Ansku
Copy link
Member

Ansku commented Jun 4, 2024

Sorry for the delay, missed the comment!

Roundtrip means a single message cycle from server to client and back -- or the other way around, if the roundtrip starts from the client, but the helper method is only for server-originated roundtrips. So in this case, the server starts the roundtrip and tells the client that the component has been added to the Grid header, and the client completes the roundtrip by sending back an acknowledgement that it has got that particular bunch of messages and has finished the immediate round of processing for all of them, which means that the component is now registered on the client as well and will be taken into account in any future size calculations. Only Grid doesn't actually send those sorts of confirmation messages from the client, it just registers the component and adds it to the DOM and is satisfied with that, so you need the helper method to learn that the processing actually did get started.

The runAfterRoundTrip is a convenience method for monitoring that server-client roundtrip without needing to write any custom extensions or JavaScript calls or adding timers/counters/listeners/whatnot to figure out what kind of messages are coming in or out and when exactly it's early enough to attempt the call without wasting any unnecessary time either -- and it also ensures that the roundtrips actually happen and complete even if turns out that the client wouldn't otherwise have anything in particular to tell the server at that time.

The helper method sends its own message to the client in the same bunch of messages with your other commands (adding the component) when the round-trip starts, and has a client-side counterpart that sends an answer from the client in the returning bunch of messages. And when that answering message arrives, it executes the callback you gave the helper method as a parameter. Even if the answering message isn't from Grid specifically, the entire bunch of messages from server to client is processed before any of the answering bunch or messages are sent back from client to server, so you know that Grid's client-side has also received knowledge of the component already.

If you try to recalculate the widths before the component has been properly added and populated, you'll probably get wrong results, hence the delay. Listening to the roundtrip is also more robust than just adding some fixed delay before triggering the recalculation, since the roundtrip adjusts to any slowness in the environment.

You can also give the runAfterRoundTrip another parameter for how many round trips you want to wait for, but that's not necessary very often. E.g. the layouting in general usually needs several rounds to finish, and there can be several server-client roundtrips during that time, but most operations don't need absolutely everything to be ready before they can be triggered. You can also give the helper method a second callback, which gets triggered if the registration gets cancelled before the first callback is triggered, e.g. because the UI got detached -- definitely not necessary in this particular use case, but might be useful for some other need.

@Tymur-Lohvynenko-Epicor
Copy link
Author

@Ansku thanks for the detailed explanation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants