If a Spotlight widget has long text in the title or the description, the display breaks at the smallest mobile widths. The title text sticks out above the top of the image, and the pager buttons overlay and obscure part of the title.

Would it be worth it to set a maximum character length on the title and description? Or is there a way to manage the display so that it doesn't look like this? See screenshot, taken at 320px width.

Screenshot showing long title text extending above the image in a spotlight widget at 320px browser width

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cboyden created an issue. See original summary.

dsnopek’s picture

Status: Active » Needs review
FileSize
1000 bytes

Here's a first pass at this. I tested it under Chrome and Firefox, and it seems to work.

It makes me a little nervous, though... I'll post specifically which bits worry me in the next comment (so I can use Dreditor).

dsnopek’s picture

  1. +++ b/panopoly-widgets-spotlight.css
    @@ -142,10 +143,12 @@
    +    margin: 0 50px -6px 0;
    

    I'm not sure why this -6px is necessary, and it's a pretty specific magic number.

    When changing to "overflow: hidden" this 6px gap just appears under the label. Why?

  2. +++ b/panopoly-widgets-spotlight.css
    @@ -153,6 +156,10 @@
    +.pane-bundle-spotlight .panopoly-spotlight-info p {
    +    max-height: 2.4em;
    +    overflow: hidden;
    +}
    

    This could break a custom theme that styles the spotlight without completely removing the panopoly-widgets-spotlight.css

    I guess the same could be said of the other changes, except this one is working on 'p' tags which are commonly styled in custom themes (whereas the rest are 'div' tags).

dsnopek’s picture

Also, since the CSS for spotlight labels is already pretty complex we should really test this under a couple IE versions as well.

dsnopek’s picture

I tested on Browserstack with IE11, IE10, IE9 and IE8.

In IE9-11, it looks perfect.

In IE8, it passable-ish -- there's actually issues with the Play/Pause buttons and Responsive Bartik, so this is definitely the least of our problems. The main difference in IE8 is that the magic gap mentioned in #3 is less then 6px (maybe 4-5px) so the label overlaps the description slightly.

This could probably use much, much more testing under more browsers and devices...

dsnopek’s picture

I tested under the following mobile browser on Browserstack (per it's suggested list to get the most coverage):

  • Apple iPhone 5s (iOS 7.0)
  • Apple iPhone 6s (IOS 9.0)
  • Google Nexus 6 (Android 5.0)
  • Samsung Galaxy S5 (Android 4.4)
  • Apple iPad Air 2 (Tablet, iOS 8.0)
  • Google Nexus 7 (Tablet, Android 4.4)

I needed some changes to prevent the label from overlapping below the spotlight, and some artifacts of the 2nd line of text in the label appearing on the Galaxy S5.

However, on the iPad Air 2, it doesn't have the magic 6px gap at all, so the label overlaps the description. :-/ If we could figure out why that gap appears on the desktop and remove the need for the -6px margin, that would be ideal! That said, it's the only browser (mobile or desktop) that I've tested so far that doesn't have the gap, so if it's really the only, only one, that might be acceptable...

dsnopek’s picture

FileSize
1 KB
591 bytes

Er, forgot to attach the patch with changes for the testing in #6!

dsnopek’s picture

FileSize
1.29 KB
1.29 KB

Here's a new patch! It should work for more font sizes without necessarily needing special theming (at least to work - not necessarily to look good ;-)). It also solves (finally!) the mysterious -6px gap issue, which makes me feel a little more comfortable about this patch in general.

I've tested it in IE13, IE11, IE9 (sort of arbitrary selection), Chrome, Firefox, Opera 36 and Safari 9.

Next up is another round of mobile testing!

dsnopek’s picture

Ok, I retested the mobile devices from #6 and they all worked great! The problem I had previously with the iPad Air 2 is no longer present. :-)

dsnopek’s picture

FileSize
1.87 KB
437 bytes

Here's an updated version that sets a min-height so that there is always room for the label and description, even if the image is broken or not loaded yet. Of course, we shouldn't have broken images, and try to make sure the images are loading fast, but now that we're clipping the title and description to the size of the widget, it started looking kinda crazy when those things happened. :-) This isn't perfect but it's an improvement.

EDIT: Here's a Travis build with this patch: https://travis-ci.org/panopoly/panopoly/builds/126643703

EDIT 2: That build used the patch from #8: https://travis-ci.org/panopoly/panopoly/builds/126715089

dsnopek’s picture

FileSize
1.54 KB

Bah! The last patch didn't merge with the 7.x-1.x branch. Here's a better version - the interdiff is still correct.

dsnopek’s picture

In order to help debug, I started #2717277: Upload screenshots to imgur on failed tests

Here's a build which is using that patch with this patch:

https://travis-ci.org/dsnopek/panopoly/builds/127204291

dsnopek’s picture

Status: Needs review » Needs work

So, for some reason this totally breaks our tests. :-/

Here's the image from the failed tests:

http://i.imgur.com/ti80UDb.png

It shows that the image or the title and description are visible. I suspect (but don't know for sure) that the image might not have been loading during the tests, but these CSS changes make it so that the title/description don't appear when the image doesn't load. In any case, we have to get this passing in order to merge it!

Here's a new travis build just in case this fixed itself in the meantime: https://travis-ci.org/panopoly/panopoly/builds/188953584

dsnopek’s picture

Status: Needs work » Needs review
FileSize
1.4 KB
554 bytes

Ok, here's another attempt at doing what I tried to do in #10/#11 that will hopefully work on Travis.

EDIT: Here's the Travis build: https://travis-ci.org/panopoly/panopoly/builds/195991711

  • dsnopek committed afde7a8 on 7.x-1.x
    Update Panopoly Widgets for Issue #2623318 by dsnopek: Spotlight title...
dsnopek’s picture

Status: Needs review » Fixed

Aha! Finally passed testing on Travis. Committing!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.