Refs T1048 - Subscriptions action added to hovercard by event listener - not for Differential & Maniphest though
Also adds form option to actions
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- Restricted Maniphest Task
Clicked that button for Pholio Mocks, got subscribed/unsubscribed. Workflow does not work D: (you get a page reload basically)
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- D5613-2
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 627 Build 627: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
| src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php | ||
|---|---|---|
| 108 | Would probably prefer to have this be 'Subscribed' but have the button inactive, like the action list on the object. | |
| src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php | ||
|---|---|---|
| 108 | Oh, thanks for spotting. This wasn't supposed to be here. | |
Comment Actions
Just kicking this back for @chad's comments.
| src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php | ||
|---|---|---|
| 100–105 | I think this one can go either way, but in general we should err on the side of showing (but disabling/deactivating) options which are possible but not currently permitted -- this lets the user click them to learn why they can't take the action. But just dropping the button in this case seems pretty reasonable. | |
| src/view/widget/hovercard/PhabricatorHovercardView.php | ||
| 44–52 | Maybe this should just be a PhabricatorActionView? Not sure how messy that would be. Would simplify the disabled/inactive stuff too. | |
Comment Actions
Updating this diff, rebasing to latest master. You guys have been very busy.
I'll go for using the action view in hover cards, coming in another diff (soonish).