Closed Bug 1527195 Opened 1 year ago Closed 1 year ago

Uneven height cards for bordered card grid with 1 line title (e.g., default basic layout)

Categories

(Firefox :: New Tab Page, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 67
Iteration:
67.1 - Jan 28 - Feb 10
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- unaffected
firefox66 + verified
firefox67 --- verified

People

(Reporter: Mardak, Assigned: thecount)

References

Details

(Keywords: github-merged, regression)

Attachments

(3 files)

Looks like description flexes but single line titles cause the card to become smaller. Not sure if expected or not ?

Flags: needinfo?(wkonu)

"expected behavior is cards in a row should have the same height, "

Flags: needinfo?(wkonu)

Bisect seems to point to bug 1519879 which did change the html for DSCard

Blocks: 1519879
Keywords: regression

I believe it's a similar issue where the <a> is no longer the direct child of the grid parent, so when the <a> tries to flex, it flexes to the now <div> parent ??

[Tracking Requested - why for this release]:

Assignee: nobody → sdowne
Iteration: --- → 67.1 - Jan 28 - Feb 10
Priority: -- → P1
Blocks: 1527504
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Status: RESOLVED → VERIFIED

Scott, can you add verification steps and needinfo Brahmini when you've posted them?

Flags: needinfo?(sdowne)

Steps to verify:

  1. Set pref http://browser.newtabpage.activity-stream.discoverystream.config/ to {"enabled":true,"show_spocs":true,"layout_endpoint":"https://getpocket.com/v3/newtab/layout?version=1&consumer_key=40249-e88c401e1b1f2242d9e441c4&layout_variant=dev-test-all"}
  2. Load about:home
  3. Look for a sponsored card that's taller than the other cards in the row.

expected: The other cards in the row's bottom should be aligned with the taller card.

I'm adding a screenshot to compare with Ed's screen shot.

Flags: needinfo?(sdowne)
Attached image align-bottom.png
Flags: needinfo?(bnagabandi)
Status: VERIFIED → RESOLVED
Closed: 1 year ago1 year ago
Flags: needinfo?(bnagabandi)

Can you review the uplift and make the request?

Flags: needinfo?(sdowne)

Wolasi, can you QA?

Flags: needinfo?(wkonu)

I have verified in Nightly 67.0a1 on Mac OS 10.14.3 by checking the height of cards in the dev-test-all layout endpoint.

Flags: needinfo?(wkonu)
Status: RESOLVED → VERIFIED

Comment on attachment 9044254 [details]
Bug 1527195 - Height issues fixed for ds card

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

None

User impact if declined

elements on the new tab page will be improperly/awkwardly sized

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

Yes

If yes, steps to reproduce

(see comment #8)

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

this is a fairly simple patch and can be QA'd visually and doesn't introduce any new code paths

String changes made/needed

n/a

Attachment #9044254 - Flags: approval-mozilla-beta?

Comment on attachment 9044254 [details]
Bug 1527195 - Height issues fixed for ds card

Planned work for pocket/new tab. Verified in Nightly.
OK for beta uplift, should land for beta 9.
Landing order: bug 1519879, bug 1525494, bug 1526861, bug 1524669, bug 1527195, bug 1525391, bug 1527347, bug 1525366, bug 1527626, bug 1527397, bug 1518258, bug 1527701, bug 1527370.

Attachment #9044254 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: needinfo?(sdowne)

(In reply to Wolasi from comment #13)

I have verified in Nightly 67.0a1 on Mac OS 10.14.3 by checking the height
of cards in the dev-test-all layout endpoint.

Can you please verify this issue on Firefox 66 Beta 9 (https://archive.mozilla.org/pub/firefox/candidates/66.0b9-candidates/build1/)?

Flags: qe-verify+
Flags: needinfo?(wkonu)

I have verified in Beta 66.0b9 on Mac OS 10.14.3 by checking the height of cards in the dev-test-all layout endpoint.

Flags: needinfo?(wkonu)

(In reply to Wolasi from comment #18)

I have verified in Beta 66.0b9 on Mac OS 10.14.3 by checking the height of
cards in the dev-test-all layout endpoint.

Thanks for verifying this!

Flags: qe-verify+
See Also: → 1531099
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.