Paths

Table of Contentst

[WIP] Masking space when only one active space
AbandonedPublic
Actions

Authored by tycho.tatitscheff on Sep 18 2015, 4:16 PM.

Details

Reviewers

epriestley
chad

Group Reviewers

Blessed Reviewers

Maniphest Tasks

T9436: Shift to Space shows on installs with no spaces
T9425: Archiving all Spaces leaves sadpanda select behind

Summary

Fixes T9425.
Fixes T9436.
As discussed in T9425, once one tested Spaces, there is no way to end the test comppletly.
In fact, some disturbing Select element remains, torturing the mind of the administrator, who tested Spaces.
Moreover it is even more disturbing for the users who have been promised that Spaces will be removed completly.

Test Plan

This is WIP, really not for direct merged.
I will do extensive test try on developpement server, once I get some review.

Diff Detail

Repository

rP Phabricator

Branch

fix/T9425

Lint

Lint Passed

Unit

Tests Skipped

Build Status

Buildable 8004
Build 9038: [Placeholder Plan] Wait for 30 Seconds
Build 9037: arc lint + arc unit

Event Timeline

tycho.tatitscheff updated this revision to Diff 34128.Sep 18 2015, 4:16 PM

tycho.tatitscheff retitled this revision from to [WIP] Masking space when only one active space.

tycho.tatitscheff updated this object.

tycho.tatitscheff edited the test plan for this revision. (Show Details)

tycho.tatitscheff added reviewers: chad, epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptSep 18 2015, 4:16 PM

Herald added a subscriber: Korvin. · View Herald Transcript

tycho.tatitscheff updated this object.Sep 18 2015, 4:17 PM

tycho.tatitscheff edited edge metadata.

tycho.tatitscheff added a task: T9425: Archiving all Spaces leaves sadpanda select behind.

tycho.tatitscheff added inline comments.Sep 18 2015, 4:21 PM

webroot/rsrc/js/application/maniphest/behavior-batch-editor.js
28	That is my jokker question. I have no clue, how make javelin mask some item depending on an `PhabricatorCursorPagedPolicyAwareQuery`. Shall i pass something in config ? And then display space or not dependingon config ?

tycho.tatitscheff added inline comments.Sep 18 2015, 4:37 PM

src/view/form/control/AphrontFormPolicyControl.php
338	This is what T9425/Q135 was about ! Masking Spaces `Select` if there is not 2 or more active Space.

Very WIP (pseudo code) test of masking Javelin Space 'Shift to space'

tycho.tatitscheff marked an inline comment as done.Sep 18 2015, 5:06 PM

tycho.tatitscheff added inline comments.

webroot/rsrc/js/application/maniphest/behavior-batch-editor.js
17–29	My solution should work ! It is maybe ugly.

tycho.tatitscheff mentioned this in Z1336: General Chat.Sep 18 2015, 5:09 PM

@epriestley, @chad : ready for a review if time !

tycho.tatitscheff added inline comments.Sep 20 2015, 10:16 AM

webroot/rsrc/js/application/maniphest/behavior-batch-editor.js
17–29	should be `select_items` or `select_rows` or `action_rows`

tycho.tatitscheff added projects: Spaces, NUX.Sep 23 2015, 8:58 AM

tycho.tatitscheff mentioned this in T9152: Plan Spaces v2.Sep 23 2015, 9:00 AM

tycho.tatitscheff mentioned this in T9436: Shift to Space shows on installs with no spaces.Sep 23 2015, 1:02 PM

tycho.tatitscheff added a task: T9436: Shift to Space shows on installs with no spaces.

tycho.tatitscheff updated this object.

@epriestley, what do you think conceptually of this ?? If you have time giving me some thought ?

@tycho.tatitscheff See Planning

@chad i'm not asking for a full review nor for merging it soon (TM) !
I was just wondering what revision I had open and thus I was asking if it could be an working way to fix T9425 and T9436.
I fully understand if you have no time to look at it.

I fully understand if you have no time to look at it.

I think there is still some miscommunication going on, or maybe the documentation we provide and point people to isn't clear. We'd prefer not to have tasks and diffs pinged for updates because: