Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
See screenshot
Comment | File | Size | Author |
---|---|---|---|
#28 | link-field.png | 58.72 KB | Maninders |
#26 | 2611268-26.patch | 1.71 KB | thpoul |
#26 | interdiff-2611268-10-26.txt | 1.52 KB | thpoul |
#19 | link_screen.png | 9.91 KB | prateekS |
#11 | seven2.png | 89.29 KB | thpoul |
Comments
Comment #2
iamDamon CreditAttribution: iamDamon at Attiks commentedComment #3
iamDamon CreditAttribution: iamDamon at Attiks commentedFollowing patch fixes the issue.
Comment #4
darketaine CreditAttribution: darketaine commentedIt looks fine.
Comment #5
attiks CreditAttribution: attiks at Attiks commented#4 If it's ok, you may set it to RTBC
Comment #6
iamDamon CreditAttribution: iamDamon at Attiks commentedFix for Bartik
Comment #7
iamDamon CreditAttribution: iamDamon at Attiks commentedAnd here's the patch
Comment #8
attiks CreditAttribution: attiks at Attiks commentedPatch has some side effects
Comment #9
darketaine CreditAttribution: darketaine commentedTrue, I think the problem isn't on the width of input, but on how browsers manipulate fieldset.
Comment #10
darketaine CreditAttribution: darketaine commentedFirefox needs another fix (display: table-cell;)
Comment #11
thpoul CreditAttribution: thpoul as a volunteer commentedLooks fine by me in both themes.
Comment #12
thpoul CreditAttribution: thpoul as a volunteer commentedComponent is wrong, it should be either Seven Theme or Bartik Theme, tagging accordingly.
Comment #13
attiks CreditAttribution: attiks at Attiks commentedLooks good
Comment #14
Wim LeersI'd like a +1 from either @LewisNyman or @emma.maria, considering this is adding a browser-specific work-around.
Comment #15
darketaine CreditAttribution: darketaine commentedAs this is on Seven theme component, Bartik is "getting lost" so I added the relative tag.
Comment #16
darketaine CreditAttribution: darketaine commentedActually as this is on both Bartik and Seven maybe the component should be changed
Comment #17
star-szrI don't think we have a CSS maintainer currently but theme system doesn't seem quite right either.
Does this also affect themes using Classy as their base? Not saying we should try to fix Classy necessarily but I'm curious.
Comment #18
darketaine CreditAttribution: darketaine commentedThe patch is specific to Bartik and Seven as I think it will be a bit 'risky' to apply it in Classy, but yes, if that change came from Classy (I checked that it has no fieldset-specific rules) then all themes based on it would be 'fixed'.
Comment #19
prateekS CreditAttribution: prateekS as a volunteer and at Srijan | A Material+ Company commentedI have tested this patch in Seven, Bartik and Stark theme and one contributed theme(adminimal_theme) too. Its working fine in all of them(have added screenshot for this contributed theme). I think we should close it.
Comment #20
darketaine CreditAttribution: darketaine commentedThis patch affects only Seven and Bartik (Adminimal is based on Seven) and no other theme (not even Stark).
Comment #21
emma.mariaI will take a look at this issue for Bartik plus advise on Seven also.
Comment #22
emma.mariaComment #23
darketaine CreditAttribution: darketaine at Pixual commentedI can see that the browser specific prefix causes a frustration.
The only reason it's added like that and not in fieldset { } it's because of
in /core/themes/seven/css/components/form.css. So I thought that's the way it should be done in context of consistency.
If it should be removed I guess it should be removed wherever it is used in core.
Comment #24
emma.mariaMozilla are actively trying to remove the
@-moz-document
browser hack...https://bugzilla.mozilla.org/show_bug.cgi?id=1035091
But they are aware that it is popularly used to help add a FF specific fix to fieldset widths and they have set this bug as a blocker for removing the browser hack.
https://bugzilla.mozilla.org/show_bug.cgi?id=504622
We are using this exact hack right now in the CSS and I think it's OK that we further add it to anything else that needs it for now.
I'm also raising a follow up as Bartik has a different field (email with multiple values) which uses
<table>
markup as it also causes a horizontal scrollbar.Comment #25
emma.mariaComments need to be added above any
@-moz-document
selectors in the patch to say that this CSS will not be needed and removed once this Mozilla bug is fixed — https://bugzilla.mozilla.org/show_bug.cgi?id=504622.Comment #26
thpoul CreditAttribution: thpoul at Pixual commentedHere you go.
Comment #27
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedComment #28
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedThis is working fine after applying patch #26. Attached screenshot for your reference(link-field.png).
Comment #29
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedComment #30
alexpottAssigning to @Cottser as he has already been involved.
Comment #33
emma.mariaThe patch still applies cleanly...
Comment #35
star-szrMigrate UI tests again as far as I can see. I sent for a retest yesterday so trying again.
Comment #37
star-szrMigrate UI. Hoping to review this tomorrow or Sunday!
Comment #38
Maninders CreditAttribution: Maninders at Srijan | A Material+ Company commentedComment #39
star-szrDidn't quite get to reviewing this - had a great workshop/discussion on Sunday at the Frontend United sprints though :)
Comment #41
star-szrThis is yucky to have to add more of these hacks but luckily it's temporary and it's something we should fix :)
Committed 3c7e318 and pushed to 8.2.x. Thanks!