[WebProfilerBundle] Improve AJAX toolbar panel #21007
Conversation
|
A quick question: when there are no Ajax requests, the entire Ajax panel is hidden. Do you really see it when you have 0 requests? |
|
Yes on hover.. and it's ugly ;-) edit: not sure if that's actually intended though? |
|
@ro0NL that's a Symfony bug. There's no Ajax panel when there are no Ajax requests:
|
|
Hmz interesting. I also noticed edit: @javiereguiluz it's a regression i guess; i cannot rebase this PR onto 3.2 in any way. Feel free to add a commit and re-fix it :) |
|
Hmm maybe we need a new tag for this "improvement"? |
|
Updated. Havent tracked back where this got broken, so im not sure how it was done before. |
|
It seems to behave the same as latest 3.2 now, only difference i spotted 3.2 handles failures a bit different;
There's no red block in 3.3 |
|
@javiereguiluz What do we do with this one? Is it a bug that should be fixed in 3.2 as well? Do we have a regression in current master (or even several as noted in the latest @ro0NL's comment)? |
|
@javiereguiluz Can you have a look at this one? |
|
@fabpot @javiereguiluz regression is caused by #20197 (see https://github.com/symfony/symfony/pull/20197/files#diff-de00cc2fe56e59a48a4d26e7a0dccca3L133) (for the red block at least) |
|
Investigated it a bit further.. The display:none not being applied initially behaves really weird; after a few refreshes it works. var requestCounter = document.querySelector('.sf-toolbar-ajax-requests');
if (!requestCounter) {
return;
}seems to false-positive sometimes. This is related to the window.load event concurring with the xhr load event. The second issue with the red status block not working is because the JS promise gets rejected due CORS policy (same as in 3.2), however we dont reach |
|
@fabpot @javiereguiluz we're all good 👍 Part of the regression was caused by a missing else case, which actually happened before (https://github.com/symfony/symfony/pull/20197/files#diff-de00cc2fe56e59a48a4d26e7a0dccca3L137) |
| @@ -498,7 +498,6 @@ | |||
| Sfjs.addEventListener(window, 'load', function() { | |||
| Sfjs.createTabs(); | |||
| Sfjs.createToggles(); | |||
| Sfjs.renderAjaxRequests(); | |||
There was a problem hiding this comment.
This file is included in toolbar_js first.. so for being super fast window.load actually wins from the XHR response :))
|
Thank you @ro0NL. |
|
@patrick-mcdougle the following https://github.com/symfony/symfony/pull/20197/files#diff-de00cc2fe56e59a48a4d26e7a0dccca3R488 caused a race condition between window & xhr load. https://github.com/symfony/symfony/pull/20197/files#diff-de00cc2fe56e59a48a4d26e7a0dccca3R259 didnt finish ajax requests in case of errors (promise failure). |
|
@ro0NL Thanks! I understand the race condition and the promise failure, but the Is there another missing else? |
|
The if from L193 had none ;-) |
…(ro0NL) This PR was squashed before being merged into the 3.3-dev branch (closes symfony#21007). Discussion ---------- [WebProfilerBundle] Improve AJAX toolbar panel | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> Before   After   Commits ------- afbcaa7 [WebProfilerBundle] Improve AJAX toolbar panel
Before
After