Fix dump panel hidden when closing a dump #24665

julienfalque · 2017-10-22T22:29:56Z

Q	A
Branch?	2.8
Bug fix?	yes-ish
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	-

In the dump panel of the debug bar, when closing a dump the panel sometimes get hidden:

This is because when the size of the panel is reduced, if the mouse is not over it anymore, the :hover pseudo-class does not apply anymore.

I "fixed" it by setting a min-height on the panel when closing a dump. The min-height is removed when leaving the panel on purpose:

For now I only tested it on Firefox 56 on Arch Linux.

nicolas-grekas · 2017-10-23T14:09:18Z

I didn't try the behavior on a page where the dump is displayed inline. Does this change anything in this situation? (eg in the dump() panel, or when using {{ dump() }})

julienfalque · 2017-10-23T14:12:06Z

Didn't try either but it's worth checking before merging. I'll have a look.

julienfalque · 2017-10-23T21:26:53Z

I updated the PR: previous version was not working with inline dumps (the panel variable was set to null with the first inline dump as the debug bar wasn't loaded yet, and then never updated).

I also noticed that all dumps have a z-index: 99999. What's the reason for that?

ogizanagi · 2017-10-24T05:21:46Z

@julienfalque : I still get a Uncaught TypeError: Cannot read property 'style' of undefined at toggle error in the profiler with your latest fix :/

julienfalque · 2017-10-24T05:48:51Z

Woops. Should be fixed now.

nicolas-grekas · 2017-10-24T12:34:35Z

src/Symfony/Component/VarDumper/Dumper/HtmlDumper.php

@@ -138,6 +138,11 @@ function toggle(a, recursive) {
    } else if (/\bsf-dump-expanded\b/.test(oldClass)) {
        arrow = '▶';
        newClass = 'sf-dump-compact';
+
+        if ('undefined' !== typeof window.Sfdump.debugBarPanel) {


This means var-dumper knows something about the Symfony panel.
Can't we decouple things and alter the logic in the panel's twig file instead?

Done. I used a custom event to decouple toggle detection and panel min-height handling.

nicolas-grekas · 2017-10-26T13:58:31Z

src/Symfony/Component/VarDumper/Dumper/HtmlDumper.php

@@ -141,6 +141,11 @@ function toggle(a, recursive) {
    } else {
        return false;
    }
+
+    s.dispatchEvent(new CustomEvent('sfbeforedumptoggle', {


breaks on IE11?

We don't care about IE for Symfony development tools (we do care about Microsoft Edge though).

we do care about IE11 for things running in the toolbar, because this one is injected in the page itself, and so anything breaking the JS could break the page itself.

note that today, IE11 supports the WDT properly (we just had an issue with the SVG rendering in the past doing wonky stuff when the size was not defined in the SVG, and we fixed it)

I disagree. Symfony debug toolbars are related to the backend. If you have a Symfony-related problem in your app, you should be able to use any browser. So there's no need to support legacy browsers. The only exception are Ajax requests ... but that's two different things: you can use IE to debug frontend + JS errors related to Ajax and any other browser to debug the Symfony responses to those Ajax requests.

At least failing gracefully should be required IMHO.

@javiereguiluz but if the frontend is broken because of the JS injected by the toolbar, you cannot debug the frontend because the toolbar causes extra errors.
It is OK if some feature does not work. It is not OK for the JS to break.

nicolas-grekas · 2017-10-28T16:28:00Z

Status: needs work

(should degrade gracefully on non-supported OSes)

julienfalque · 2017-10-30T18:31:32Z

Rewritten using document.createEvent(). Support by older browsers should be better. Though I cannot confirm it right now. Can someone have a look?

nicolas-grekas · 2017-11-05T15:14:06Z

src/Symfony/Component/VarDumper/Dumper/HtmlDumper.php

@@ -141,6 +141,15 @@ function toggle(a, recursive) {
    } else {
        return false;
    }
+
+    var event = doc.createEvent('Event');
+    event.initEvent(


this call should be on one line.
to further enhance compat, what about wrapping the new logic in a check like: if (doc.createEvent) {...}?

nicolas-grekas · 2017-11-05T15:49:52Z

src/Symfony/Component/VarDumper/Dumper/HtmlDumper.php

+        event.initEvent('sf-dump-expanded' === newClass ? 'sfbeforedumpexpand' : 'sfbeforedumpcollapse', true, false);
+    }
+
+    s.dispatchEvent(event);


should be inside the "if"?

Oops, indeed... Also added to the if condition.

fabpot · 2017-11-05T16:13:49Z

Thank you @julienfalque.

This PR was merged into the 2.8 branch. Discussion ---------- Fix dump panel hidden when closing a dump | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes-ish | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - In the dump panel of the debug bar, when closing a dump the panel sometimes get hidden: ![before](https://user-images.githubusercontent.com/1736542/31867025-615e9c48-b788-11e7-8329-96716c211523.gif) This is because when the size of the panel is reduced, if the mouse is not over it anymore, the `:hover` pseudo-class does not apply anymore. I "fixed" it by setting a min-height on the panel when closing a dump. The min-height is removed when leaving the panel _on purpose_: ![after](https://user-images.githubusercontent.com/1736542/31867054-d01a01cc-b788-11e7-9ef7-8418ae2b3094.gif) For now I only tested it on Firefox 56 on Arch Linux. Commits ------- 2e0b263 Fix dump panel hidden when closing a dump

carsonbot added Status: Needs Review Bug labels Oct 22, 2017

nicolas-grekas added this to the 2.8 milestone Oct 23, 2017

carsonbot removed the Status: Needs Review label Oct 23, 2017

nicolas-grekas approved these changes Oct 23, 2017

View reviewed changes

carsonbot added the Status: Reviewed label Oct 23, 2017

ogizanagi approved these changes Oct 23, 2017

View reviewed changes

julienfalque force-pushed the fix-dump-panel-toggling branch 2 times, most recently from b1d290f to b16ffd2 Compare October 23, 2017 21:23

julienfalque force-pushed the fix-dump-panel-toggling branch from b16ffd2 to ece2e1f Compare October 24, 2017 05:48

ogizanagi approved these changes Oct 24, 2017

View reviewed changes

javiereguiluz approved these changes Oct 24, 2017

View reviewed changes

nicolas-grekas reviewed Oct 24, 2017

View reviewed changes

julienfalque force-pushed the fix-dump-panel-toggling branch 2 times, most recently from 55a7515 to 4b58a83 Compare October 24, 2017 22:56

nicolas-grekas reviewed Oct 26, 2017

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Oct 28, 2017

julienfalque force-pushed the fix-dump-panel-toggling branch from 4b58a83 to 70ba05a Compare October 30, 2017 18:24

nicolas-grekas reviewed Nov 5, 2017

View reviewed changes

julienfalque force-pushed the fix-dump-panel-toggling branch from 70ba05a to 2b9b6e0 Compare November 5, 2017 15:37

nicolas-grekas reviewed Nov 5, 2017

View reviewed changes

Fix dump panel hidden when closing a dump

2e0b263

julienfalque force-pushed the fix-dump-panel-toggling branch from 2b9b6e0 to 2e0b263 Compare November 5, 2017 15:52

carsonbot removed the Status: Needs Work label Nov 5, 2017

nicolas-grekas approved these changes Nov 5, 2017

View reviewed changes

carsonbot added the Status: Reviewed label Nov 5, 2017

fabpot approved these changes Nov 5, 2017

View reviewed changes

fabpot merged commit 2e0b263 into symfony:2.8 Nov 5, 2017

julienfalque deleted the fix-dump-panel-toggling branch November 5, 2017 17:30

This was referenced Nov 5, 2017

Release v3.4.0-BETA3 #24831

Merged

Release v4.0.0-BETA3 #24832

Merged

Release v2.8.29 #24915

Merged

Release v3.3.11 #24916

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dump panel hidden when closing a dump #24665

Fix dump panel hidden when closing a dump #24665

julienfalque commented Oct 22, 2017 •

edited

Loading

nicolas-grekas commented Oct 23, 2017

julienfalque commented Oct 23, 2017

julienfalque commented Oct 23, 2017

ogizanagi commented Oct 24, 2017

julienfalque commented Oct 24, 2017

nicolas-grekas Oct 24, 2017

julienfalque Oct 24, 2017

nicolas-grekas Oct 26, 2017

javiereguiluz Oct 26, 2017

stof Oct 26, 2017

stof Oct 26, 2017

javiereguiluz Oct 26, 2017

nicolas-grekas Oct 26, 2017

stof Oct 26, 2017

nicolas-grekas commented Oct 28, 2017

julienfalque commented Oct 30, 2017

nicolas-grekas Nov 5, 2017

julienfalque Nov 5, 2017

nicolas-grekas Nov 5, 2017

julienfalque Nov 5, 2017

fabpot commented Nov 5, 2017

Fix dump panel hidden when closing a dump #24665

Fix dump panel hidden when closing a dump #24665

Conversation

julienfalque commented Oct 22, 2017 • edited Loading

nicolas-grekas commented Oct 23, 2017

julienfalque commented Oct 23, 2017

julienfalque commented Oct 23, 2017

ogizanagi commented Oct 24, 2017

julienfalque commented Oct 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolas-grekas commented Oct 28, 2017

julienfalque commented Oct 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Nov 5, 2017

julienfalque commented Oct 22, 2017 •

edited

Loading