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
Details
- Reviewers
epriestley chad - Maniphest Tasks
- T7582: Implement policies in Conpherence to support rooms
- Commits
- Restricted Diffusion Commit
rP7e0c5162763c: Conpherence - add edit control for rooms
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
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)
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. |
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...