-
-
Notifications
You must be signed in to change notification settings - Fork 818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix dev/core#5561 - CMS theme floats break FormBuilder layout #31457
base: 5.80
Are you sure you want to change the base?
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
FYI @artfulrobot and @vingle this will need to be ported to other themes. |
@totten I see you added a label to run additional tests, but note this is a css-only patch, and our css files have no test coverage, regardless of cms. The test runs on this PR are basically irrelevant. |
@colemanw Yeah, agree the unit-tests don't matter for CSS patch. However, the (Alas, the failure seems to be a pretty persistent problem in |
oh yes, I remember having to fix this in Aah (pre-Thames/RiverLea). Relevant commits:
Your CSS will apply to all |
Yeah I was wondering if it was only an issue in FormBuilder interface if it's safer to preface with FWIW I can't see this issue on RiverLea so maybe this was already fixed. Do you know which CMSs this specifically is a problem with so I can test there? |
I've just taken a look with Drupal + Seven and legends are broken there too, in a way that this fix doesn't fix (position: absolute, then re-aligned - below in Greenwich). Other than using legends less – which I'd support given how quirky and awkward to style legends+fieldset are (with the fieldset border wanting to sit in the middle of the legend) I think there needs a more comprehensive legend reset, ie
|
@vingle I've updated this PR with your suggested change :) |
Based on the multiple complaints about this and the simplicity of the fix I've changed the base to target version |
@artfulrobot regarding this question, BS3 clearly assumes that these |
Assuming those 6 lines are added at the bottom of the file (sorry, not familiar with this stuff), this didn't work. The result was exactly the same as before. System is 5.78.3 under Backdrop 1.29.1. I cleared caches using Civi and browser cache. In Display preferences 'Greenwich' is set for both backend and frontend (it was automatic). In Backdrop themes the theme is set to 'Seven' for Civi Admin. BTW If I set the Civi Admin theme to 'Basis' it fixes the problem (before and after applying this fix) so there is a workaround. |
@clarkac1 sorry this is a #bootstrap-theme legend {
position: initial;
text-transform: inherit;
float: none;
} |
Overview
Fix CMS theme from breaking FormBuilder layout.
Before
After
Technical Details
I don't know if this is the best place to put this. Doing it here, it'll need to be copied into every other theme as well. But I don't think we have any "central" place for these things, so I guess we just have to copy it.