Opened 9 years ago
Last modified 2 years ago
#32577 reviewing defect (bug)
Customizer QUnit tests not cleaning up
Reported by: | iseulde | Owned by: | dlh |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.3 |
Component: | Customize | Keywords: | has-patch needs-refresh |
Focuses: | javascript | Cc: |
Description
Ideally everything should be removed from the DOM after the test or module is done.
For now we can hide it to the left instead of top, it just moves down again as more tests are added.
Attachments (5)
Change History (18)
#2
@
9 years ago
- Owner set to valendesigns
- Status changed from new to assigned
I can take a look tomorrow. I need to get more familiar with the QUnit tests anyhow.
#5
@
9 years ago
- Owner changed from valendesigns to dlh
- Summary changed from Customizer tests not cleaning up to Customizer QUnit tests not cleaning up
#6
@
9 years ago
- Keywords has-patch added; needs-patch removed
32577.patch attempts to remove the problematic controls after their respective modules complete.
I would appreciate feedback on what the desired visual experience is when viewing the test suite. The patch reverts to the positioning that was there before [32701]. In my testing, this results in the controls being briefly visible when the suite loads. top: -10000
removes the flash; removing the .css()
line altogether causes the controls to move down as @iseulde described, but eventually they disappear.
#7
@
9 years ago
Sorry: 32577.2.patch contains the pre-[32701] positioning. I left a 0
in the first one.
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
#10
@
8 years ago
- Keywords needs-patch added; has-patch removed
- Owner changed from westonruter to dlh
@dlh I tried 32577.2.patch and with the removal of left
I see the fixture UI showing up still (see screenshot above). Restoring the negative left
causes it to go away. Idea: what if the #customize-theme-controls
element had a fixed 1px square viewport with the overflow hidden? Then we wouldn't have to worry about the size of the elements in the DOM. So something like:
position: absolute; top: -10px; left: -10px; width: 1px; height: 1px; overflow: hidden;
#11
@
8 years ago
@westonruter Thanks for reviewing the patch. I see the same issue with it as in your screenshot. I'll try to figure out what happened and test your CSS idea, too.
#12
@
8 years ago
- Keywords has-patch added; needs-patch removed
32577.3.patch implements the CSS approach from 32577#comment:10, which seems pretty straightforward and works as expected in my testing.
The patch also attempts to supplement the CSS with a different method of cleaning up the DOM: Rather than attempting to pick off individual elements -- some of which appear to be created separately from of the tests themselves, if I followed correctly? -- it removes all of them at the end of the test suite, with one exception.
Of course, this approach still doesn't meet the goal of removing fixtures after each module or test. Plus, once you get to QUnit.done()
, using $( '#customize-theme-controls' ).remove()
would accomplish the same thing.
#13
@
2 years ago
- Keywords needs-refresh added
- Milestone set to Future Release
@dlh is this still occurring today? 32577.3.patch is also stale and needs a refresh if the answer is yes.
In 32701: