Opened 9 years ago
Closed 8 years ago
#34344 closed defect (bug) (fixed)
Expanded section margin-top glitches when other section is deactivated
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 4.2 |
Component: | Customize | Keywords: | has-patch |
Focuses: | javascript | Cc: |
Description
After having expanded a section, if you then deactivate another section, the margin-top
for the expanded section causes the contents to go out of view. Additionally, deactivated section is incorrectly animated when it should just collapse instantly since it is not in view.
To reproduce the problem in Twenty Fifteen, execute the following from the console:
wp.customize.section('header_image').expand( { completeCallback: function() { _.delay( function(){ wp.customize.section('colors').active(false); }, 1000 ); } } );
Video depicting the above behavior: https://cloudup.com/cP5JgMgkqFu
Relates to #33567.
Attachments (4)
Change History (24)
#2
@
9 years ago
Hey,
I found another issue with margin-top overflow,
- Drop a widget into an area
- Quickly go up level before widget populates into preview window (before ajax has finished i guess?)
- Come back into the widgets area
- margin-top starts from the top, as if it were top: 0 and ignores the saving box area
attached image above
#3
@
9 years ago
See also #34391 for a potential definitive solution to all of the margin-top
glitches.
#5
@
9 years ago
- Owner set to westonruter
- Status changed from new to assigned
Assigning to @westonruter for 4.5 tracking, since we're almost halfway through beta.
#6
@
9 years ago
- Owner changed from westonruter to ryankienstra
@ryankienstra @delawski This is closely related to what you've been working on for #35947.
In fact, I would have thought that @ryankienstra's PR would have fixed the issue as it describes:
This occurs when there are many panels, using the code provided in the ticket to produce the panels. The non-active sections and panels still occupied space in the DOM. So the active panel had a margin-top property to move it to the top. This PR sets the height of the non-active sections to 0. And it doesn't add a margin-top value to the active panel's content.
Nevertheless, the issue also happens with 334473c checked out.
Your experience here is appreciated. If it's not something that can be figured out over the next several days, this is probably something that we'll need to punt for the next release.
#7
@
9 years ago
@ryankienstra @delawski I suppose the setting the height of inactive sections to zero is irrelevant because what this involves is an active section. So then the solution it seems would be, still, to either eliminate the use of margin-top
to position the panels (#34391)?
#8
@
9 years ago
- Keywords has-patch added; needs-patch removed
Possible Solution
Hi @westonruter,
Could you please see my pull request and identical patch?
They set the height
to 0
for all sections in the root panel that aren't open.
Reason
Like we saw in trac-35947, non-open sections still took up space in the DOM. And they caused problems with the calculation of the margin-top value.
For example, this ticket gives code to reproduce the bug:
wp.customize.section('header_image').expand( { completeCallback: function() { _.delay( function(){ wp.customize.section('colors').active(false); }, 1000 ); } } );
This opens the header_image
section, and de-activates the colors
section. But the colors
section had a height of 43px
. So when it's de-activated, it is given display: none
. And it's taken out of the DOM.
But the margin-top
of the header_image
content area was already set at -259px
. So when the colors
section moves out of the DOM, the header_image
section moves too high.
This is similar to @delawski's commit for trac-35947.
#9
@
9 years ago
@ryankienstra I've cherry-picked your patch to amend it onto the patch for #35947 so that they can more easily be tested together.
This ticket was mentioned in Slack in #core by westonruter. View the logs.
9 years ago
#11
@
9 years ago
Status Update
Now Attached To trac-35947
As @westonruter suggested, this defect should be grouped with a related defect:
trac-35947 : Customizer panel fails to fully expand leaving extra margin
@westonruter cherry-picked this ticket's commit into that ticket's pull request. It looks like trac-35947 is almost ready to be merged into Beta, pending a minor issue.
This ticket was mentioned in Slack in #core by chriscct7. View the logs.
9 years ago
#13
@
9 years ago
@delawski + @westonruter is this going to be ready in time or should the ticket be punted out?
#14
@
9 years ago
- Keywords 4.6-early added
- Milestone changed from 4.5 to Future Release
As with #35947, it looks like we'll need to re-visit this early 4.6.
[attachent:34344.diff] is a work-in-progress. It eliminates the deactivation animation of the non-visible section, but there is still an issue (shown in section-margin-top-problem.png) where the
margin-top
of the section's contents still gets erroneously shifted up ~45px. Help wanted.