Page MenuHomePhabricator

Add Disabled mode to landing revision ui
ClosedPublic

Authored by avivey on Jan 30 2014, 12:16 AM.
Tags
None
Referenced Files
F14028465: D8105.diff
Fri, Nov 8, 1:21 PM
F14005465: D8105.id18338.diff
Sun, Oct 27, 1:29 PM
F14005244: D8105.id18330.diff
Sun, Oct 27, 9:50 AM
F13990994: D8105.id.diff
Tue, Oct 22, 8:00 AM
F13985101: D8105.diff
Sun, Oct 20, 6:01 PM
F13979529: D8105.id18338.diff
Sat, Oct 19, 5:01 AM
Unknown Object (File)
Oct 7 2024, 12:29 PM
Unknown Object (File)
Oct 7 2024, 12:29 PM

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).