Page MenuHomePhabricator

Remove a TODO comment
ClosedPublic

Authored by joshuaspence on Jan 3 2015, 12:48 AM.
Tags
None
Referenced Files
F19817556: D11165.diff
Thu, Mar 5, 1:52 PM
F19815457: D11165.id26814.diff
Wed, Mar 4, 9:14 PM
F19775066: D11165.diff
Sat, Feb 21, 8:19 PM
F19535514: D11165.diff
Jan 21 2026, 6:46 AM
F19535513: D11165.diff
Jan 21 2026, 6:46 AM
F19151542: D11165.id26811.diff
Dec 10 2025, 10:29 PM
F19148081: D11165.id26811.diff
Dec 10 2025, 5:14 PM
F19148047: D11165.diff
Dec 10 2025, 5:12 PM

Details

Summary

Ref T6852. Apparently, this is not a TODO.

Test Plan

N/A

Diff Detail

Repository
rP Phabricator
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 3507
Build 3515: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Remove a TODO comment.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
epriestley edited edge metadata.
This revision is now accepted and ready to land.Jan 3 2015, 12:49 AM
chad added inline comments.
src/view/form/control/AphrontFormControl.php
225

You could also add $classes[] = 'grouped'; here, but some members of the team may frown on that.

This revision was automatically updated to reflect the committed changes.

I think there may be a reason that "grouped" wasn't used here -- IIRC, it cuts off tokenizer dropdowns and the calendar date selection popup thing.

Oh grouped doesn't hide overflow, it should have been fine. At least a quick test and those cases still work. I think grouped is just "clearfix" without older browser support if I recall (like IE6, IE7, IE Mac, etc).