ryancramerdesign / ProcessWire

Our repository has moved to https://github.com/processwire – please head there for the latest version.
https://processwire.com
Other
727 stars 201 forks source link

Inputfield column height matching interferes with CKEditor #1426

Open Toutouwai opened 9 years ago

Toutouwai commented 9 years ago

It seems that these lines of JS code cause some odd bugs with CKEditor fields in Firefox and IE.

More info and screenshots in this thread at the forums.

The spacer div that is inserted to match column heights is a tricky wee thing (I've logged some issues with it in the past here at Github and in the forums). I'm wondering if the approach to matching column heights is more complicated than it needs to be. Unless I'm missing something the spacer is there for purely aesthetic reasons.

As an experiment I tried removing the spacer div via a CSS display:none rule. Of course this results in gaps when column heights are unequal, but the form is still perfectly usable. I then set the background colour of the parent ".Inputfields" ul to the same colour as the field borders and the result was pretty decent.

To go a step further: If you wanted to retain the look of a border with a lighter tinted background then the ul could be set to position:relative and an additional empty child li added just for aesthetics, with CSS position:absolute; top:0; right:0; bottom:0; left:0; background:rgba(228, 235, 238, 0.8); border:1px solid #d7e2e6;. Some z-index rules might need to be added to make sure it stays at the bottom of the stack behind the other lis. Update: we don't even need the empty child li. Just set the following for the parent ul...

background: #eaeff2;
box-shadow: 0 0 0 1px #d7e2e6 inset;

This approach will need some fine-tuning but it has the advantage that it avoids the need for all the tricky spacer div JS.

ryancramerdesign commented 9 years ago

I wasn't able to duplicate the issue in Firefox exactly as described in your forum post, but went ahead with pushing some changes to this that improved the behavior, in particular with CKEditor and image fields, especially when paired next to each other. Please let me know if you find it to be an improvement? You may have to hard-refresh to get the new JS file to load, as the PW core version hasn't yet changed this week.

You are right that maybe we don't need the spacers and could settle on using background colors in the UL, but I really do prefer to match the actual Inputfield heights from a visual perspective. The solution has also been quite reliable, other than certain combinations or situations that are relatively rare. And I try to accommodate those when they come up, like the one you've mentioned. I know spacers aren't ideal, but there isn't anything complex or tricky about them really, they are quite simple. Still we may revisit the use of them down the road, but for now want to stick with using them since they are compatible with existing admin themes and maintain the appearance we're shooting for. But if a CSS solution appears that enables us to maintain the same look, without major underlying changes (like switching to flexbox), we'll definitely want to take a close look at that.

Toutouwai commented 9 years ago

Thanks Ryan. Your commit does fix the bug in Windows Firefox, but not in IE11 - essentially the bug remains unchanged there, although I seemed to be able to key in a few more characters before the cursor jumped outside the editor window.

I'm curious to see how much of the admin theme appearance can be retained using CSS alone (without any JS spacing). Will have a tinker and report back.

Toutouwai commented 8 years ago

Ryan, I'm experiencing this issue again in v2.7.2 stable, or some closely related issues.

When a neighbouring field column or fieldset column is taller than the CKEditor field...

In Windows Firefox:

In IE11:

Toutouwai commented 8 years ago

Actually, looking at some sites running older versions of PW (but after the attempted fix) I'm not sure this fix was ever successful. I often don't notice it during development because I tend to load text first and images second, so the CKEditor field is initially taller than the neighbouring field/fieldset and the bug doesn't occur. But it becomes a problem when I or clients come back to do later edits to a page.

Toutouwai commented 8 years ago

I think the problem is here Line 703: $container.hide(); Line 705: $container.show(); Commenting out these two lines fixes the problem. Are they necessary? I didn't notice any ill effects of removing them.

isellsoap commented 8 years ago

@ryancramerdesign Is this issue really fixed yet?