Page MenuHomePhabricator

Automatically phix phrequent spelling phlubs phor all phabricator phorm phields
Needs RevisionPublic

Authored by jcox on Apr 1 2017, 5:46 PM.
Tags
None
Referenced Files
F12857621: D17594.diff
Fri, Mar 29, 10:42 AM
F12853186: D17594.diff
Fri, Mar 29, 8:07 AM
F12852478: D17594.id42323.diff
Fri, Mar 29, 7:24 AM
Unknown Object (File)
Thu, Mar 21, 8:45 AM
Unknown Object (File)
Thu, Mar 21, 8:45 AM
Unknown Object (File)
Thu, Mar 21, 8:42 AM
Unknown Object (File)
Thu, Mar 21, 7:42 AM
Unknown Object (File)
Sat, Mar 16, 9:27 AM
Tokens
"Dat Boi" token, awarded by amp343."Pterodactyl" token, awarded by bjshively."Pirate Logo" token, awarded by chad."The World Burns" token, awarded by johnny-bit."Party Time" token, awarded by timhirsh.

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Summary

We've phrequently run into the issue oph users misspelling certain words (ie. they spell "Phabricator" as "Fabricator" or "Maniphest" as "Manifest"). While these mistakes are understandable, they do cause a phair amount oph organizational conphusion around terminology. This diphph adds some Very Good Javascript Code™ to phix this automatically by correcting our users' phailed spelling attempts phor them. I think phurther work could be done to generalize this more iph we ever want to standardize our linguistic phramework in other ways.

As-is, this corrects spelling in all textarea and input[type=text] elements. This seems to work well phor all phorm phields. Aphter using phabricator with this update for about phive minutes I can't imagine possibly going back.

Test Plan

It's javascript so it doesn't need to be tested.

Diff Detail

Repository
rP Phabricator
Branch
AutophixSpellingErrors (branched from master)
Lint
Lint Skipped
Unit
No Test Coverage
Build Status
Buildable 16249
Build 21590: Run Core Tests
Build 21589: arc lint + arc unit

Event Timeline

This will make it diphphicult to log in iph your username contains an 'f'. Should I add a database migration to automatically phix peoples' usernames?

Will this also phix my password?

  • This should be wrapped in a behavior to, e.g., future-proof it against Quicksand.
  • I'm not totally sure if target will always exist in all browsers. If you don't focus anything and type a key, maybe there's no target? But maybe it's always document or something. Maybe just do an if (!target) { return; } out of general fear if this isn't scoped more narrowly (but see below for scoping).
  • This shouldn't fire if r.metaKey || r.altKey || r.ctrlKey are set on the raw event.
  • This probably shouldn't fire if e.getSpecialKey() returns anything.
  • keypress might be a better trigger event than keydown? Not sure. Behavior for holding "f" might be better one way or the other.
  • Without narrower scoping, this could misfire in a lot of cases, like, as noted, logging in. Although I think we'd all agree that these cases are better for everyone on the balance in the long run, we could limit how initially disruptive this feature is by introducing it with narrower scoping first. After winning hearts and minds, we can expand coverage and should get less pushback. An initial scoping of ['transaction-append', 'tag:textarea'] should capture events in (modern) comment textareas without hitting the login input, the Passphrase key material input, etc.
  • Does this interact smoothly with the cursor position? If not, you can use TextAreaUtils to:
    • Split the value into "before selection range", "in selection range", and "after selection range" sections.
    • Fix text in each section, counting the number of replacements.
    • Increment the selection start by the number of replacements in the first range, and the selection end by the number of replacements in the first two ranges.
    • Finally, set the new selection range.
    • The, the selection state should be stable. It's possible that this somehow already works aright.
  • Is there a /i flag in JS for case-insensitive? I think there is?
  • Maybe spend 30 seconds seeing if you can exempt UIs with a pattern like ://\S+|f to further ease the transition into this new era?
  • Because this change is so deeply visionary -- bursting at the seams with thought leadership, really -- I'd plan to deploy it to just this install for a while before shipping it to users.
This revision now requires changes to proceed.Apr 2 2017, 1:53 PM