Page MenuHomePhabricator

Conpherence - add edit control for rooms
ClosedPublic

Authored by btrahan on Apr 1 2015, 11:11 PM.
Tags
None
Referenced Files
F13103597: D12252.diff
Sat, Apr 27, 11:10 AM
Unknown Object (File)
Mon, Apr 22, 10:35 PM
Unknown Object (File)
Thu, Apr 11, 7:09 AM
Unknown Object (File)
Sat, Apr 6, 10:09 PM
Unknown Object (File)
Fri, Apr 5, 8:36 PM
Unknown Object (File)
Thu, Mar 28, 2:53 PM
Unknown Object (File)
Mar 17 2024, 7:16 PM
Unknown Object (File)
Mar 4 2024, 9:58 PM
Subscribers

Details

Summary

Fixes T7582. Basically if its a room we should be able to change title + policy and if its a thread just the title. T7582 had ideas to do a dropdown but "view in column" doesn't make sense from conpherence afaik - what would the page you'd end up with the column be? (maybe home?) Anyway, that is iteration we can add laters

Test Plan

edited room metadata successfully from main and column view. edtied thread title from main and column view.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Conpherence - add edit control for rooms.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added reviewers: epriestley, chad.
src/applications/conpherence/controller/ConpherenceUpdateController.php
170–173

I could put that "non update, try cancel" bit here instead of a re load

just error as before (tested a non updated in column and it breaks with the redirect code / is tricky to redirect correctly anyway)

epriestley edited edge metadata.
epriestley added inline comments.
src/applications/conpherence/controller/ConpherenceUpdateController.php
170–174

How do you reach this? This feels like it shouldn't be happening.

For "edit"-like forms, I'd expect non-updating edits to just go through. If you "edit task" and then "save", for example, we just accept the edit (setContinueOnNoEffect(true) on the editor). We only complain on comment workflows, generally.

This revision is now accepted and ready to land.Apr 1 2015, 11:27 PM

It just kicks you to home with the column open. Mostly column is very hidden and undiscoverable. There should be some means of discovering the feature in Conpherence.

re: toggle discoverability, the conpherence column toggle is part of the keyboard shortcut instructions on every page. I figure we can also write / update a user manual too for Conpherence.

Anyway, those dropdowns have proven tricky in the past, with the js on js nature of conpherence, so I'll add it as a separate diff next if you want. The other difference of note is the policy icon is in the title nowadays so it also looked a bit cluttered to me when I was first starting out. Just an FYI in case factors in (and / or you want the title policy icon gone from rooms too.)

src/applications/conpherence/controller/ConpherenceUpdateController.php
170–174

Perhaps line 158 is the key? AFAIK the returned $xactions are the ones that actually had effect, so it ends up being an empty array in the no effect case.

To reproduce, hit edit, hit submit.

I think a more general fix is to make Conpherence able to handle a "non update" response. T7318 is the bug for when that isn't caught in the business logic like with this little "gem" we're discussing.

I think it's reasonable to not expose the column for now. It's still a little buggy on its own, and it enables Quicksand which has like 200 bugs that I don't really want reports about.

I just had an idea when we are really ready to launch column.

Let's default it open for all new users / if you don't have the preference set, and the first time you toggle it closed we give you a heavy handed confirm dialogue that tells you how to re-open it, etc?

Yeah I didn't expect it to be discoverable today, but more when we launch it. Hiding in the menu was likely a bad idea anyways, but it was sort of consistent with the column menu.

I'll think about it and put up a few mocks...

This revision was automatically updated to reflect the committed changes.