Improve Phriction page move dialog
ClosedPublic
Actions

Authored by epriestley on Nov 12 2014, 2:22 PM.

Details

Reviewers

btrahan

Maniphest Tasks

T5492: Make wiki move dialog more intuitive and validate input

Commits

Restricted Diffusion Commit
rP120a7d916447: Improve Phriction page move dialog

Summary

Fixes T5492. I figured this would be easier to just fix than write a guide for; it actually took me an hour, but I spent like 75% of that futzing with my editor.

The Move controller currently accepts either a slug or an ID. I can't find any callsites which pass a slug, and this doesn't make sense. Pretty sure this was copy/pasted from Edit or something. Only accept IDs.
Slightly modernize the Move controller (newDialog(), handleRequest(), $viewer).
When the user enters a bad slug, warn them that we're going to fix it for them and let them accept or reject the changes.
Don't prefill the edit note (this feels inconsistent/unusual).
On the form, label the input "Path" instead of "URI".
Show the old path, to help remind the user what the input should look like.
When a user tries to do a no-op move, show a more tailored message.
When the user tries to do an overwriting move, explain how they can fix it.
When normalizing a slug like /question/???/mark/, make it normalize to /question/_/mark.

Test Plan

Tried to move a document to itself.
Tried to overwrite a document.
Did a bad-path move, accepted corrected path.
Did a good-path move.
Did a path move with a weird component like /???/.
Added and executed unit tests.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 26014.Nov 12 2014, 2:22 PM

epriestley retitled this revision from to Improve Phriction page move dialog.

epriestley updated this object.

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

epriestley added a reviewer: btrahan.

epriestley added a task: T5492: Make wiki move dialog more intuitive and validate input.

Herald added a subscriber: epriestley. · View Herald TranscriptNov 12 2014, 2:22 PM

Harbormaster completed remote builds in B3025: Diff 26014.Nov 12 2014, 2:22 PM

epriestley mentioned this in T5492: Make wiki move dialog more intuitive and validate input.Nov 12 2014, 2:30 PM

Nice, thanks for tackling this. I've got a small thing for the wiki now from polishing it up...

The edit note used to be pre-filled with the previous edit's note before I got in there. e.g.

edit 1: change title with note "changing title"
edit 2: move the page and see note as "changing title"

anyhoo, I like it best as you have it, but that's the mini history here as I understand it. =D

src/applications/phriction/controller/PhrictionMoveController.php
35	usable -> using
37–39	Should this logic also be in the editor?

This revision is now accepted and ready to land.Nov 12 2014, 2:54 PM

Yeah, I think the original note behavior was contributed and might have been copy-pasted or something (given that the slug part looks like it was copy/pasted).

src/applications/phriction/controller/PhrictionMoveController.php
37–39	Ideally maybe-yes, but I think it's fairly hard to raise a custom prompt from within the editor. I generally feel OK about putting minor quality-of-life things like this outside the editor if it's unlikely that Conduit callers would make the mistake. But if we hit a couple more of these, maybe it's an argument for making it easier to raise ad-hoc prompts from within the editor. In this case, it would also be reasonable for the Conduit version to not normalize the slug, and then the editor to reject it outright (this may already happen). Then Conduit callers would get an error ("you passed a bad slug, fix it"), which seems like the ideal behavior (trying to do a prompt over Conduit seems iffy).