Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
Content gets hidden in the vertical tabs see the screencast. This only happens in the seven theme.
Uses formtips to create a tooltip but can't escape the box due to overflow: hidden
Proposed resolution
- Remove overflow hidden
- check for regressions.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#15 | 3028675-15-seven-vertical-tabs-hidden.patch | 1.34 KB | bskibinski |
#14 | 3028675-14-seven-vertical-tabs-hidden.patch | 1.31 KB | bskibinski |
#11 | 3028675-11-seven-vertical-tabs-hidden.patch | 1.32 KB | bskibinski |
#10 | chosen-fields-broken.png | 54.84 KB | bskibinski |
#10 | 3028675-10-seven-vertical-tabs-hidden.patch | 1.32 KB | bskibinski |
Comments
Comment #2
joelpittetComment #3
joelpittetHere's a patch. It's only relevant for the seven theme which makes it easier to commit. Also doesn't seem to be any visual regression, I saw some bullets showing in the past.
Adding contrib blocker because it's breaking formtips, which is what I used in the screencast in the issue summary.
Comment #4
joelpittetIntroduced here: #1989488: Vertical tabs style update, with no specific mention to the overflow: hidden.
Comment #5
occupantPatch applies cleanly, overflow issue is no longer occurring.
Comment #6
gnuschichten CreditAttribution: gnuschichten at ETECTURE GmbH commentedI can confirm: The patch applies without errors and fix the overflow issue.
Also I can't find any usage for the property overflow:hidden.
Comment #7
alexpott#4 says there is no specific mention of overflow hidden on #1989488-3: Vertical tabs style update but there is in comment 3.
We need to verify that what is was fixing is not broken. From the comment it says:
Comment #8
joelpittet@alexpott, thanks for noticing that I didn't look hard enough. If you zoom in really close you can see the issue.
Here's an over-exaggerated version to see what is meant:
https://jsfiddle.net/t97gwhvq/
One possible solution is to round the inside elements with background color, which considering the design of this should be fairly simple.
Comment #10
bskibinskiNew patch:
If I may be so bold, I would like to see this committed as fast as possible (after review of course ;-).
The rounded corners issue, while annoying, is not breaking functionality. But as you can see in my screenshot, a lot of functionality can break because of this. In my case, chosen fields are hardly usable.
If any minor styling bugs occur because of the removal of overflow:hidden, i would recommend opening new bugs-tickets to fix those, but get this in as fast as possible, so sites don't break, function over form :-)
Comment #11
bskibinskiI've noticed it's better to put the border-radius on the
<a>
element itself instead of the<li>
.now the 'not-active' first menu item is also properly rounded.
Comment #12
Ekvo CreditAttribution: Ekvo commentedRecent patch works nice. Overflow issue is fixed, borders for seven theme are nice as well.
Comment #13
lauriiiSorry for the delayed response. I was on vacation for the last 2 weeks 🏖
Is there a specific reason we can't specify ltr styles by default, and then just override them in the rtl styles?
Comment #14
bskibinskiNo worries, hope you enjoyed your vacation! Welcome back to reality ;-)
No, none other that I personally never like to override styles if I can prevent it, but that's just pedantic me.
But you are right, it's safer/better to just have it as a general rule, and override it for the RTL.
Fixed in the new patch.
Comment #15
bskibinskiI forgot to unset the border-top-left radius for the RTL languages after the latest change in the last patch.
Fixed in patch #15.
Comment #16
Ekvo CreditAttribution: Ekvo commentedLast change tested. Borders still looking good on Seven theme with RTL language selected.
Comment #17
alexpottThese need a
/* LTR */
comment after them. Can be added on commit.Comment #18
alexpottCommitted aa4dda4 and pushed to 8.8.x. Thanks!
This is required because the
overflow: hidden
was hiding the bullets.Fixed on commit.
Also fixed on commit.
Comment #21
alexpottDiscussed with @catch and @laurii and we agreed that since this is seven only backport is allowed.