Summary:
Fixes T12538. Instead of hiding "closed" results unless only closed results match, show closed results but sort them to the bottom.
This fixes the actual issue in T12538, and I think this is probably the correct/best behavior for the global search.
It also makes all other typeaheads use this behavior. They currently have a "bug" where enabled user `abcd` makes it impossible to select disabled user `abc`. This manifests in some real cases, where enabled function `members(abc)` makes it impossible to disabled user `abc` in some function tokenizers.
If ths feels worse, we could go back to filtering in the simpler cases and introduce a rule like "show closed results if only closed results would be shown OR if query is an exact match for the disabled result", but that gets dicier because "exact match" is a fuzzy concept.
(There are a lot of other minor bad behaviors that this doesn't try to fix.)
Test Plan:
Enabled project "instabug" no longer prevents bot user "instabug" from being shown:
{F4903843}
Disabled user "mmaven" is sorted below enabled user "mmclewis", in defiance of the otherwise alphabetical order. There's no visual cue that this user is disabled because of T6906.
{F4903845}
Same as above, but this source renders "disabled" in a more obvious way:
{F4903848}
Function selecting members of active project `members(instabug)` no longer prevents selection of bot user `instabug`:
{F4903849}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12538
Differential Revision: https://secure.phabricator.com/D17695
Summary:
Ref T8510. We had two issues with mixed-case result sorting, like typing `@joe` to match user `Joe`.
- The fallback sort was not normalized properly, so "J" could sort after "j". Instead, normalize values for sorting.
- The `prefix_hits` and older `priority_hits` mechanisms were competing destructively. The `prefix_hits` mechanism completely replaces the `priority_hits` mechanism. Instead, use only the `prefix_hits` mechanism.
Test Plan:
- Copied results for "joe" from WMF.
- Hard-coded the controller to return them.
- Searched for `@joe`.
- After patches, first hit is user "Joe".
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8510
Differential Revision: https://secure.phabricator.com/D16826
Summary:
Ref T8510. Sort prefix matches above non-prefix matches, so that "Ape Discovery" does not match "discovery" better than "Discovery".
Sort functions last.
Rename function internal strings so they don't get over-promoted the prefix-match rules.
Add kind of a hack to get "Project X" sorting above all the "Project X (Milestone 1)" results.
Test Plan:
Created "Ape Discovery", "Baboon Discovery", "Chimpanzee Discovery", etc.
Main project now sorts above milestones:
{F1681773}
Prefix matches now sort above other matches:
{F1681774}
Function results (rarely used) are now less prominent:
{F1681775}
Better function results here:
{F1681776}
More function results:
{F1681777}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8510
Differential Revision: https://secure.phabricator.com/D16094
Summary:
Ref T10163.
- If you click a result, we get a blur before your click hits, and deactivate before the click can work. Instead, wait before responding to blur.
- Use the standard sort handler which puts unixnames over human names. Also use the standard filter which deals with disabled users not matching unless they're the only match.
Test Plan:
- Clicked a result, got a replacement.
- Named myself "dog dog", typed "@dog", user "@dog" was now first match despite me being "@admin".
- Used normal typeaheads to make sure I didn't break sort handler.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10163
Differential Revision: https://secure.phabricator.com/D15031
Summary: Ref T3725. This probably has 900,000 bugs. This will need updates for subprojects/milestones.
Test Plan:
- Tested very gently in Safari, Firefox and Chrome.
- Reasonable inputs appear to work.
- Clicking, escape, tab, return, arrow keys work OK?
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T3725
Differential Revision: https://secure.phabricator.com/D15029
Summary: Fixes T9871. Ref T10004. These won't win any awards but it fixes them being incredibly weird and confusing.
Test Plan:
{F1029090}
- Tried to use controls, got reasonable behavior.
- Used normal controls to make sure I didn't break anything.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9871, T10004
Differential Revision: https://secure.phabricator.com/D14814
Summary:
Ref T9132. Fixes T4580. Thhat might actually have been fixed a while ago or something since it describes a buggy/bad interaction which doesn't reproduce for me at HEAD.
This saves and restores all the stacked actions (subscribers, projects, etc) so that you don't lose anything if you close a window by accident.
Test Plan:
Added a bunch of actions in various states, reloaded the page, draft stuck around.
Submitted form, actions didn't stick around anymore.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4580, T9132
Differential Revision: https://secure.phabricator.com/D14675
Summary:
Ref T9132. This is kind of a mess because the tokenizer rewrite left rendering tokenizers in Javascript a little rough. This causes bugs like icons not showing up on tokens in the "Policy" dialog, which there's a task for somewhere I think.
I think I've fixed it enough that the beahavior is now correct (i.e., icons show up properly), but some of the code is a bit iffy. I'll eventually clean this up properly, but it's fairly well contained for now.
Test Plan:
- Reassigned a task.
- Put a task up for grabs.
- No reassign on closed tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9132
Differential Revision: https://secure.phabricator.com/D14669
Summary: These values aren't always provided. They should be, but just make things work for now.
Test Plan:
- Edited a task.
- Selected "Custom Policy" for "Edit Policy".
- Configured a policy like "Users: dog".
- Saved policy.
- Selected "Custom Policy" again to edit the policy.
- Before patch: tokenizer fails to render with an error (`config.types` is not an object).
- After patch: tokenizer renders properly, albeit with a missing icon.
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D12477
Summary:
Ref T4100. See D12465.
- Instead of putting CSS color classes on the tokenizer icons, put them on the container tags.
- Note that this removes the "bluegrey" default classes.
- This doesn't actually add CSS for the classes so, e.g., "green" doesn't make things green yet. This just supports D12465.
Test Plan: Viewed markup, saw classes.
Reviewers: chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12467
Summary:
Ref T4100. Overall:
- Use token background color to communicate token type (blue = object, yellow = function, grey = disabled/closed, red = invalid).
- Use token icon color to make color choices consistent (specifically, use project icon colors in project tokens).
- For functions, use token icon to communicate function result type (e.g., viewer() has a user icon; members(...) has a group icon), since we don't need the icon to indicate "this is a function" anymore.
Test Plan:
{F374615}
{F374616}
{F374617}
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12446
Summary:
Ref T4100. This is still a bit rough around the edges, but mostly does what we're after.
- Implements viewer() and members(...) functions.
- The new browse workflow makes these discoverable.
Test Plan: {F374201}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: chad, epriestley
Maniphest Tasks: T4100
Differential Revision: https://secure.phabricator.com/D12444
Summary: Ref T5750. This makes browse work for all of the dynamic tokenizers in Herald, Policies, batch editor, etc.
Test Plan: Used tokenizers in Herald, Policies, Batch editor.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5750
Differential Revision: https://secure.phabricator.com/D12442
Summary: Ref T5750. This adds a basic browse view. Design is a bit rough, see T7841 for some screenshots.
Test Plan: Used browse view to add tokens to tokenizers.
Reviewers: chad, btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5750
Differential Revision: https://secure.phabricator.com/D12441
Summary: Fixes T6102. Gets the from D10867 running in the tokenizer. This thing is pretty copy / pastey, but I guess that's okay?
Test Plan: looked at a project //tokenizer// and typed "project". since I have a million things with the word "project" in it, I was delighted to see the "project" project first in this project tokenizer.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T6102
Differential Revision: https://secure.phabricator.com/D11305
Summary: Fixes T5659. When building a token after a user selection, we currently use the `value` as the token text, but sometimes that's an internal name which doesn't make much sense to users. For projects, it is now "sluga slugb Proper Display Name". If available, use `displayName` instead.
Test Plan: Typed some projects into a tokenizer, got display names only.
Reviewers: chad, joshuaspence
Reviewed By: joshuaspence
Subscribers: epriestley
Maniphest Tasks: T5659
Differential Revision: https://secure.phabricator.com/D9996
Summary:
Ref T4420. Fixes T5306. Currently, the main menubar search has a lot of redundant/unshared code.
Move some common functions into `JX.Prefab.whatever()` and call them from the main search.
The major change here is that we apply the same "only show closed/disabled/archived objects if there are no matching open objects" logic, fixing T5306.
Test Plan:
- Used normal typeaheads.
- Used global search.
- Searched for a prefix shared by open and archived projects, didn't see the archived ones until the open ones were exhausted.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Maniphest Tasks: T5306, T4420
Differential Revision: https://secure.phabricator.com/D9899
Summary:
Updates policy, headers, typeaheads to FA over policy icons
Need advice - can't seem to place where icons come from on Typeahead? Wrong icons and wrong colors.... it is late
Test Plan:
- grepped for SPRITE_STATUS
- grepped for sprite-status
- grepped for setStatus for headers
- grepped individual icons names
Browsed numerous places, checked new dropdowns, see pudgy people.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: epriestley, Korvin
Maniphest Tasks: T4739
Differential Revision: https://secure.phabricator.com/D9179
Summary: The "Assign / Claim" tokenizer in Maniphest prefills with the viewer,
but doesn't have an icon right now. This causes an issue; make the `icon` key
optional.
Auditors: btrahan
Summary: Ref T4420. I think the border is good enough without this spinny thing, and it needs a lot of code.
Test Plan: Used typeahead.
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8249
Summary:
Ref T4420. When a result list contains both open and closed results, hide the closed results. I think this has a good chance of almost always working, and feeling very intuitive. It has a small chance of being a weird mess. It feels reasonable to me so far
The one bad case I can come up with here is that if you have results which shadow each other, like "Apples" (a closed project) and "Apples and Bananas" (an open project), it is impossible to get "Apples" in the result list, because "Apples and Bananas" will always shadow it. Let's wait for someone to hit this before we figure out how to deal with it.
Test Plan: Typed through open stuff to hit closed stuff.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8238
Summary:
Ref T4420. Fixes T3309. Two major UX issues here:
- When the user extends a query ("alin" -> "alinc"), we currently hide all the results, then show them again when the new results arrive. This makes the typeahead feel a bit flickery. Instead, show matching results, then add more results when everything arrives.
- When loading more results from ondemand sources, we currently do not give you any indication that things are loading. Instead:
- Show a loading GIF (this might need #design help, @chad).
- Slightly lighten the control border.
- I didn't want to do anything like actually add "loading" text because it would cause UI flicker in the 'extend a query' case and some other cases, but otherwise this design is totally made up.
Test Plan: Typed into tokenizers and extended queries, got a better-feeling UI.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T3309, T4420
Differential Revision: https://secure.phabricator.com/D8233
Summary:
Ref T4420. Tokenizers currently operate in "preload" or "ondemand" modes. In the former mode, which is default, they'll try to load the entire result list when a page loads.
The theory here was that this would slightly improve the experience for small installs, and once they got big enough they could switch to "ondemand". In practice, several issues have arisen:
- We generally don't have a good mechanism for telling installs that they should tweak perf config -- `metamta.send-immediately` is the canonical example here. Some large installs are probably affected negatively by not knowing to change this setting, and having settings like this is generally annoying.
- We have way way too much config now.
- With the advent of ApplicationSearch, pages like Maniphest make many redundant loads to prefill sources like projects. Most of the time, this data is not used. It's far simpler to switch everything to ondemand than try to deal with this, and dealing with this would mean creating two very complex divergent pathways in the codebase for a mostly theoretical performance benefit which only impacts tiny installs.
- We've been using `tokenizer.ondemand` forever on `secure.phabricator.com` since we have many thousands of user accounts, and it doesn't seem sluggish and works properly.
Removing this config is an easy fix which makes the codebase simpler.
I've retained the ability to use preloaded sources, since they may make sense in some cases (in at least one case -- task priorities -- adding a static source pathway might make sense), and they're part of Javelin itself. However, the code will no longer ever go down that pathway.
Test Plan: Used `secure.phabricator.com` for years with this setting enabled.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8232
Summary:
Ref T4420. This is mostly a design change, but addresses two functional issues:
# Many sources exclude disabled accounts, system agents, archived projects, etc. It is rare to select these, but excluding them completely is too severe, and we've made more than a handful of changes over time to replace a "users" endpoint with an "accounts" endpoint (to include disabled users) or similar. Instead, always show these results, but sort them last and use a special style to clearly mark them as closed, disabled, or otherwise unusual.
- As a practical consequence, all the similar endpoints can now be merged, so "accounts" and "users" return the exact same result sets.
# Increasingly, sources can return multiple object types in a single list. For example, "CC" can have a user or mailing list, and soon a project or repository. However, the result list is fairly homogenous across types and it isn't easy to quickly pick out projects vs users. To help with this, add icons showing the result type.
Test Plan:
{F113079}
(The main search results get touched here too, I verified they didn't blow up.)
Reviewers: chad, btrahan
Reviewed By: chad
CC: chad, aran, mbishopim3
Maniphest Tasks: T4420
Differential Revision: https://secure.phabricator.com/D8231
Summary:
Ref T1279. The new dual-mode user/project tokenizers are a bit disorienting. Provide content type hints.
Very open to any suggestions here, most of this patch is just getting the right data in the right places. We can change things up pretty easily.
- I like the little icons in the tokens themselves, I think they look good and are useful.
- I'm less sold on the '(Project)' thing I did in the dropdown. We can easily make this richer if you have thoughts on it -- we could put icons in the left column maybe? Or right-justify the types?
- I made it always sort users above projects.
Test Plan: See screenshot.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: chad, aran, carl
Maniphest Tasks: T4420, T1279
Differential Revision: https://secure.phabricator.com/D7250
Summary: If `jsxmin` is not available, use a pure PHP implementation instead (JsShrink).
Test Plan:
- Ran `arc lint --lintall` on all JS and fixed every relevant warning.
- Forced minification on and browsed around the site using JS behaviors. Didn't hit anything problematic.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5670
