@sgiehl opened this Pull Request on November 26th 2021 Member

Description:

When a report containing some kind of datatable is loaded, the delivered code also includes some javascript to initialize the datatable (see _dataTableJS.twig). We used to simply initialize all (new) datatables found on the page. This actually led to #18373. When switching the date range, all reports on the goals page are reloaded. In most cases the evolution chart is finished first. This chart already triggers a datatable initialize. In most cases the detail report below is already loaded but not yet rendered into the page. The javascript already tries to initialize the datatable, resulting in the weird behavior.

This PR will change this, so only the report contained in the response (based on the reportid) will be initialized.
Locally that seems to work fine for me.

fixes #18373

Review

@sgiehl commented on November 26th 2021 Member

Seems the UI tests are passing so far. There are a couple of failures that are unrelated though. All those are already fixed on 4.x-dev. So fixing them here might result in even more merge conflicts when merging next_release back...

@tsteur commented on November 28th 2021 Member

👍

@sgiehl commented on November 29th 2021 Member

@tsteur thanks for the feedback. I've applied some changes.

@tsteur commented on November 30th 2021 Member

@sgiehl I just tested this with a range and I still had the issue:

image

I double checked that there was no browser caching.

@sgiehl commented on December 2nd 2021 Member

@tsteur moved some js calculation to simpler css usage. hope that should fix it finally. At least I wasn't able to reproduce in local testing.

@sgiehl commented on December 3rd 2021 Member

@tsteur I actually had a closer look at those tests. The full page of the pivot test looks like this:
image

So the reason that it looks cut off, is actually that the resolution in tests is too low, to display the full size. Before it was displayed with a fixed width of 1200px, causing the full page to be scrollable, and that way it had enough space to be displayed fully.

The screenshots that still needs to be updated, are located in a submodule. Will do that now, before merging this

This Pull Request was closed on December 3rd 2021
Powered by GitHub Issue Mirror