Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I often experience the problem with the Save button on the end of a admin page to be cut, so it's hard to see and hard to hit (especially since in some browsers the URL of a link you hover is shown in that area as well).
See screen shot: http://screencast.com/t/u9qXwKmfGfpl
The screen show is taken on a site where I have scrolled down to the bottom of the page. It's from Firefox, but the problem is in Chrome, Safari and probably other browsers as well.
It can be solved by changing the padding in modules/overlay/overlay-child.css from
#overlay {
display: table;
margin: 0 auto;
min-height: 100px;
min-width: 700px;
position: relative;
padding: .2em;
padding-right: 26px;
width: 88%;
}
to:
#overlay {
display: table;
margin: 0 auto;
min-height: 100px;
min-width: 700px;
position: relative;
padding: .2em 26px 2em .2em;
width: 88%;
}
Comment | File | Size | Author |
---|---|---|---|
#35 | overlay-padding-1153122-34.patch | 681 bytes | Mac_Weber |
#32 | overlay-padding-1153122-32.patch | 417 bytes | Mac_Weber |
#25 | padding.png | 27.74 KB | Mac_Weber |
#16 | drupal-1153122-16.patch | 744 bytes | tim.plunkett |
#10 | 1153122-10-added-padding-to-overlay.patch | 742 bytes | Mac_Weber |
Comments
Comment #1
yoroy CreditAttribution: yoroy commentedThanks for the report, looks like an issue with overlay module.
Comment #2
Mac_Weber CreditAttribution: Mac_Weber commentedpreparing the patch
Comment #3
Mac_Weber CreditAttribution: Mac_Weber commentedComment #4
Mac_Weber CreditAttribution: Mac_Weber commentedComment #6
Mac_Weber CreditAttribution: Mac_Weber commented#3: added-padding-to-overlay-1153122-2.patch queued for re-testing.
Comment #8
Mac_Weber CreditAttribution: Mac_Weber commentedComment #9
Mac_Weber CreditAttribution: Mac_Weber commentedComment #10
Mac_Weber CreditAttribution: Mac_Weber commentedComment #11
Dave ReidComment #12
Mac_Weber CreditAttribution: Mac_Weber commentedComment #13
Mac_Weber CreditAttribution: Mac_Weber commentedComment #14
cweagansYour efforts in battling the Testbot were valiant. I think this patch is ready to go.
Comment #15
Mac_Weber CreditAttribution: Mac_Weber commentedThanks everybody at #drupal-contribute for helping! This is my first patch.
I think now I learned the tricks.
Comment #16
tim.plunkettAdded leading 0 to decimal values.
s/ (\.\d)/ 0\1/g
No patch credit, Mac_Weber did all the work. Just trying to keep the CSS clean.
Comment #18
cweagans#16: drupal-1153122-16.patch queued for re-testing.
Comment #20
Bojhan CreditAttribution: Bojhan commentedCan we have some compareable screenshot?
Comment #21
cweagansLooks like we're all green, and the patch in #16 looks good. I don't think a screenshot is needed for this one. It's just adding a bit of padding to the bottom of the overlay.
Comment #22
Mac_Weber CreditAttribution: Mac_Weber commentedwhy #16 reports success, but #17 reports 'failed testing' for the same file?
Comment #23
cweagansI didn't see the error before it was retested - it was likely that it was a "failed to checkout drupal" or "failed to apply patch" error. It's all green now though, so nothing to worry about - all the tests pass and this change is not going to break anything.
Comment #24
Bojhan CreditAttribution: Bojhan commentedIt needs a comparable screenshot, please don't deprive me from reviewing this issue - it did not get a visual review, it is my job to give one.
Comment #25
Mac_Weber CreditAttribution: Mac_Weber commentedScreenshot:
Comment #26
aspilicious CreditAttribution: aspilicious commentedPlease wait until overlay rtl is comitted. This needs rtl styling. Thnx! :)
Comment #27
Mac_Weber CreditAttribution: Mac_Weber commentedHow it could affect rtl?
Comment #28
cweagansThis is not blocked on rtl. If this goes in first, the rtl patch needs to account for this patch. If rtl goes in first, this patch should account for rtl.
Comment #29
aspilicious CreditAttribution: aspilicious commentedThis *was* blocked on that patch. It got commited yesterday. Now this patch doesn't apply anymore. And yes it needs to be tested in rtl too.
Comment #30
Mac_Weber CreditAttribution: Mac_Weber commented@aspilicious the only patch related to rtl I see commited is http://drupal.org/node/766458 (Seven theme lacks rtl styling), which clearly states: 2) This patch doesn't fix the overlay, thats a seperate issue
This patch looks good to go.
Comment #31
aspilicious CreditAttribution: aspilicious commentedWebchick forgot to commit a file in http://drupal.org/node/766170
But either way this pathc doesn't apply anymore because that one is fixed.
Srry this is needs work...
Comment #32
Mac_Weber CreditAttribution: Mac_Weber commentedtested with RTL.
Comment #33
aspilicious CreditAttribution: aspilicious commentedIs 26px the same as 0.2em?
Probably not so this needs RTL styling.
Why mixing pixels and em? I would choose em. So 26px needs to be converted.
23 days to next Drupal core point release.
Comment #34
Mac_Weber CreditAttribution: Mac_Weber commentedThat 26px were added for RTL in http://drupal.org/node/766170
That's why I did not change it.
Comment #35
Mac_Weber CreditAttribution: Mac_Weber commentedadded bottom padding separated, as other commits to D8.x had done to other paddings
Comment #36
cweagansThat looks good. I think this should apply correctly to both D7 and D8.
Comment #37
Dries CreditAttribution: Dries commentedCommitted to 7.x and 8.x. Thanks.
Comment #38
cweagansCongrats on your first core patch, Mac_Weber! Patch got applied to two branches, too! :)