Closed Bug 1023914 Opened 6 years ago Closed 6 years ago

Empty state image cut off

Categories

(Firefox for Android :: Awesomescreen, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 33
Tracking Status
firefox31 --- unaffected
firefox32 --- verified
firefox33 --- verified
fennec 32+ ---

People

(Reporter: Margaret, Assigned: Margaret)

References

Details

Attachments

(2 files)

ibarlow ran into this: http://cl.ly/image/451j2x2s2k0Z

Looks like a regression from bug 996708. That patch isn't mission critical, so we could just back it out to start.
(In reply to :Margaret Leibovic from comment #0)
> ibarlow ran into this: http://cl.ly/image/451j2x2s2k0Z
> 
> Looks like a regression from bug 996708. That patch isn't mission critical,
> so we could just back it out to start.

Is this bug Nightly-specific? It seems like the fix would be to simply set scaleType to fitCenter.
tracking-fennec: ? → 32+
The problem is that there was padding taking up part of the image height. Since this space only exists to put space between the image and the text, replacing the padding with a margin fixes the issue.
Attachment #8439628 - Flags: review?(lucasr.at.mozilla)
(In reply to Lucas Rocha (:lucasr) from comment #1)
> (In reply to :Margaret Leibovic from comment #0)
> > ibarlow ran into this: http://cl.ly/image/451j2x2s2k0Z
> > 
> > Looks like a regression from bug 996708. That patch isn't mission critical,
> > so we could just back it out to start.
> 
> Is this bug Nightly-specific? It seems like the fix would be to simply set
> scaleType to fitCenter.

I would prefer to just have the image match the size of the ImageView, since the point of the patch in bug 996708 was to provide guidelines about what image size developers should use. However, I suppose we could also change the scaleType to prevent developers from accidentally ending up with cut off images if they use the wrong image size. What do you think?
(In reply to :Margaret Leibovic from comment #3)
> (In reply to Lucas Rocha (:lucasr) from comment #1)
> > (In reply to :Margaret Leibovic from comment #0)
> > > ibarlow ran into this: http://cl.ly/image/451j2x2s2k0Z
> > > 
> > > Looks like a regression from bug 996708. That patch isn't mission critical,
> > > so we could just back it out to start.
> > 
> > Is this bug Nightly-specific? It seems like the fix would be to simply set
> > scaleType to fitCenter.
> 
> I would prefer to just have the image match the size of the ImageView, since
> the point of the patch in bug 996708 was to provide guidelines about what
> image size developers should use. However, I suppose we could also change
> the scaleType to prevent developers from accidentally ending up with cut off
> images if they use the wrong image size. What do you think?

I agree about setting guidelines for add-on developers but I think we should try to ensure the UI doesn't look too broken if guidelines are not followed. fitCenter would at least ensure we never show cut off images. I don't feel strongly about it, up to you.
Attachment #8439628 - Flags: review?(lucasr.at.mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/0137da4202c5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Device: Alcatel One Touch
OS: Android 4.1.2
Build: Firefox for Android 33.0a1 (2014-06-16)
The empty state image is not cut anymore.
Comment on attachment 8439628 [details] [diff] [review]
Turn empty view image padding into margin

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 996708 
User impact if declined: empty view image cut off
Testing completed (on m-c, etc.): landed on m-c 6/15
Risk to taking this patch (and alternatives if risky): low-risk tweaks to layout for empty add-on panels
String or IDL/UUID changes made by this patch: none
Attachment #8439628 - Flags: approval-mozilla-aurora?
Comment on attachment 8439628 [details] [diff] [review]
Turn empty view image padding into margin

Aurora approval granted.
Attachment #8439628 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Device: LG Nexus 4 (Android 4.4.2)
Build: Firefox for Android 32.0a2 (2014-06-19)
The empty state image is not cut anymore.
Status: RESOLVED → VERIFIED
Depends on: 1046579
You need to log in before you can comment on or make changes to this bug.