Page MenuHomePhabricator

Add Disabled mode to landing revision ui
ClosedPublic

Authored by avivey on Jan 30 2014, 12:16 AM.
Tags
None
Referenced Files
F14077893: D8105.diff
Fri, Nov 22, 1:21 AM
Unknown Object (File)
Thu, Nov 21, 12:47 AM
Unknown Object (File)
Sat, Nov 16, 7:00 AM
Unknown Object (File)
Mon, Nov 11, 6:45 PM
Unknown Object (File)
Fri, Nov 8, 1:21 PM
Unknown Object (File)
Sun, Oct 27, 1:29 PM
Unknown Object (File)
Sun, Oct 27, 9:50 AM
Unknown Object (File)
Oct 22 2024, 8:00 AM

Details

Summary

Fixes T4066.

add isActionDisabled() to DifferentialLandingStrategy, which also explains why it is so.
Make an appropriate pop-up in the controller.

Also make the whole UI "workflow", and convert createMenuItems() to createMenuItem() (Singular).

Test Plan

Click "Land to..." button in all kinds of revisions.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

src/applications/differential/controller/DifferentialRevisionLandController.php
110โ€“113

we can maybe get rid of this test here, because it's the nth time we run it, but it's also the first time in this particular request.

src/applications/differential/landing/DifferentialLandingStrategy.php
13

Did we had a reason for this to be pluralized? It made extracting the "disabled" state yucky.

37โ€“38

I don't like this pattern much, but it's short for an object with getDisabledStatus():bool and getDistabledReason():string.

Seems reasonable to me.

src/applications/differential/landing/DifferentialLandingStrategy.php
13

Given that you could implement multiple strategies to add multiple items, de-pluralizing it seems reasonable.

Closed by commit rPd0e30882a11f (authored by @avivey, committed by @epriestley).