Opened 8 years ago
Closed 8 years ago
#39349 closed defect (bug) (fixed)
Customizer (mobile preview) site title extra padding
Reported by: | halftones | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.7.1 | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | Customize | Keywords: | has-patch commit fixed-major |
Focuses: | Cc: |
Description
Customizer adds extra 10px left padding to site title when preview in mobile mode.
Attachments (2)
Change History (15)
#2
@
8 years ago
My theme not using selective refresh, and i don't see any edit shortcuts. I just see this padding and only in mobile mode.
#3
@
8 years ago
@westonruter Oh, I didn't realize there was body padding being added. Yeah, the shortcuts definitely shouldn't affect the page layout if at all possible, and certainly not if they're not even present. (At the very least I think that sort of global adjustment styling should be up to the theme.)
Adding a class name to partial containers is a clever solution that sounds like it would prevent this padding from being applied without a partial.
I wonder, however, if the padding is needed at all. A quick test of a few default themes without that padding looks ok to my eye (although we'd definitely have to do a more careful analysis to see what that was meant to fix).
#4
@
8 years ago
@sirbrillig didn't you add the padding so that the edit shortcuts wouldn't get cut off at the edge of the screen?
#5
@
8 years ago
@westonruter it's... possible? But I *think* that was added by someone else. There were a lot of contributions to the CSS of that patch. I've never been particularly concerned about the icons being cut off. I think it's a better experience than changing the layout to accommodate them. Regardless of who wrote it originally, I don't think it's a good solution.
#7
@
8 years ago
@westonruter nice find! :-) Looks like I copy-and-pasted those rules from this patch https://core.trac.wordpress.org/attachment/ticket/38651/38651.5.diff into the next one without looking very carefully.
I still think it's not wise for the edit shortcut styles to move anything in the preview if there's any way to prevent it.
I'll drop a patch here to strip those styles out and we can see what it looks like.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#10
@
8 years ago
- Focuses performance removed
- Keywords commit added; needs-screenshots removed
The customizer should not be modifying any theme styles directly; any compensating for edit shortcuts needs to happen on those elements or within themes themselves.
+1 for 39349.diff, which should be in 4.7.1 if possible and looks ready for commit. Removing needs-screenshots
since the "before" screenshot clearly illustrates the padding that's being removed by the patch.
#11
@
8 years ago
- Owner set to westonruter
- Resolution set to fixed
- Status changed from new to closed
In 39685:
@halftones thanks for the report. This is to accommodate the visual edit shortcuts. If your theme implemented selective refresh to preview changes to the title then you'd see an edit shortcut there in the preview. Note also that if you collapse the controls pane the padding should be removed, since the edit shortcuts are also then removed.
@sirbrillig Should we extend the base
Partial
JS logic to add a class name to all containers for partial placements? And then this class name can be used in the selectors to add this additional padding.