Summary: This is unused and there's no way to even set it. Pretty sure it died a while ago when we switched app icons to FontAwesome.
Test Plan: Grepped for `appIcon`, found no related hits. Browsed some pages.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15074
Summary: We plan to use these more in future mocks. Adds base colors and re-uses in Phame.
Test Plan: Phame, mobile and desktop.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15069
Summary:
Ref T10054. This tries to make the members page a bit more consistent and provide hints to users about subproject/milestone membership rules. In particular:
- You now join, leave, watch, unwatch, add and remove members, and lock and unlock membership from the members screen.
- We now explain the membership rule for the project on this screen. There are currently four rules:
- Normal Project: Join/leave normally.
- Parent Project: Uses subprojects to determine members.
- Milestone: Uses parent project to determine members.
- Locked: Membership is locked.
- (Future) Imported from LDAP/other external sources: Membership is determined by something else.
Test Plan: {F1064878}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15059
Summary:
Ref T10054. I think this gets everything except:
- circles on icons;
- I spent ~15 minutes poking at animations but wasn't able to get anything that looked reasonable whatsoever.
Test Plan:
- Collapsed menus.
- Expanded menus.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15056
Summary: These are old project image choices, remove and only go with FontAwesome related images.
Test Plan: Project -> Edit Picture -> Save
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15051
Summary: Removes header gradient images for flat, CSS controlled colors. I didn't convert the "pony" colors over, going with few options for easier theme-ability.
Test Plan:
Test each color choice.
{F1063828}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15052
Summary: Icon in a circle. Base class, not much in the way of color choices.
Test Plan:
UIExamples, Chrome.
{F1062027}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15034
Summary: Ref T10054. People, projects, and instances no longer use icon nav, so we can get rid of it in favor of profile menus.
Test Plan: Grepped for everything, looked through workboards/profiles again.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15022
Summary:
Ref T10054. This makes the profile menu full-height. It uses two pieces of dark magic:
- `calc()`, which allows you to do math in CSS.
- The `vh` unit, which is CSS for "viewport height".
Apparently this kind of stuff just works now? CSS got good at some point?
Test Plan:
- Page looks correct in Safari, Chrome, Firefox.
- Checked `caniuse.com` for `vh` and `calc()`, saw they're supported?
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15017
Summary:
Ref T10054. I haven't done any of the big-picture layout stuff yet, but this should get look-and-feel somewhere in the ballpark of reasonablness, I think.
Major missing stuff:
- No "collapse" state or action yet.
- Menu is not full-height (requires changes to the rendering pipeline).
Test Plan: {F1060941}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15016
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: Ref T10054. Just simplifying this a bit before I start laying in the new profile menus.
Test Plan:
- Viewed Diviner on desktop and checked the mobile menu.
- Viewed Files on desktop and checked the mobile menu.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15015
Summary: Ref T10054. This mostly makes sure that mobile gets to have the same profile menu that Desktop does.
Test Plan:
- Viewed menus on mobile, saw all profile menu actions available.
- Viewed/used changed pages (column detail, edit column, edit picture).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15008
Summary:
Fixes T9323. Two minor fixes:
- On the first commit, don't render a downward line.
- Clean up a 1px spacing issue that had cropped up a while ago when we added icons or something, I think.
Test Plan:
Before:
{F1057248}
After:
{F1057249}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9323
Differential Revision: https://secure.phabricator.com/D14974
Summary:
When looking at a large file in Diffusion:
- disable highlighting if it's huge and show a note about why;
- pick up a few other optimizations.
Test Plan: Locally, this improves the main render of `__phutil_library_map__.php` from 3,200ms to 600ms for me, at the cost of syntax highlighting (we can eventually add view options and let users re-enable it).
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14959
Summary:
Ref T10077. Ref T8918. The way the main menu is built is not very modular and fairly hacky.
It assumes menus are provided by applications, but this isn't exactly true. Notably, the "Quick Create" menu is not per-application.
The current method of building this menu is very inefficient (see T10077). Particularly, we have to build it //twice// because we need to build it once to render the item and then again to render the dropdown options.
Start cleaning this up. This diff doesn't actually have any behavioral changes, since I can't swap the menu over until we get rid of all the other items and I haven't extended this to Notifications/Conpherence yet so it doesn't actually fix T8918.
Test Plan: Viewed menus while logged in, logged out, in different applications, in desktop/mobile. Nothing appeared different.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8918, T10077
Differential Revision: https://secure.phabricator.com/D14922
Summary: Ref T4245. Pass the whole repository in so it can do something else in a future change.
Test Plan: Loaded changesets in Diffusion.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14931
Summary: Creates a new next/previous UI for PhamePosts, and adds a setFoot to PHUIDocumentViewPro for future use in other apps.
Test Plan:
Test first, next, last posts on Phame in mobile, desktop, and tablet breakpoints.
{F1050152}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14919
Summary: Ref T10004. This control doesn't disable visually or behaviorally, e.g. when locked in an EditEngine configuration.
Test Plan:
- Locked field for Projects.
- Reviewed form in EditEngine.
- Created/edited a project.
- Swapped default.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14911
Summary: I'll revert this within 24 hours, before I promote.
Test Plan: Had completely functional but greatly enhanced Phabricator experience in Firefox, Safari and Chrome.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14884
Summary: Ref T5240. We don't currently use any of these options and I don't think we have any plans to use them. Strip them out for now to make fixing drag-and-drop stuff easier.
Test Plan: Grepped for removed stuff, no hits.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5240
Differential Revision: https://secure.phabricator.com/D14864
Summary:
Ref T10004. This could sometimes pass `false`, which counts as disabled.
Instead, pass `null` explicitly.
Test Plan: Edited default space on an EditEngine form.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14858
Summary:
Fixes T1895. Now that we have on-demand prviews, we can use them on mobile. On mobile:
- don't show live previews;
- only save drafts every 10 seconds.
Also, show fewer remarkup buttons on mobile to try to make sure the more important ones (preview, e.g.) fit.
Test Plan:
- Made window narrower and wider to trigger preview/no-preview behavior.
- Used DarkConsole to verify request rate.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T1895
Differential Revision: https://secure.phabricator.com/D14856
Summary:
Ref T3967. This gives us a reasonable baseline for doing remarkup previews inline in all contexts, and works in weird/constrained context including:
- inline comments;
- conpherence; and
- custom fields.
It would be nicer to go beyond this in contexts like Phame posts, but this is a start, at least.
Test Plan:
{F1040877}
{F1040878}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T3967
Differential Revision: https://secure.phabricator.com/D14855
Summary: Currently it's difficult to notice posts may be much longer than the summary (in most cases, actually), Adds a consistent "Read More" link to the full post. Also made sure "violet" was more consistantly used.
Test Plan:
Review summaries in Phame
{F1031799}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14827
Summary: Ref T10004. Happy to take another approach here or just not bother, this just struck me as a little ambiguous/confusing.
Test Plan:
Before, not necessarily clear that the "Create Task" header only applies to the first few items.
{F1029126}
After, more clear:
{F1029127}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14815
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 T10004. After D14804, we get this behavior by default and no longer need to set it explicitly.
(If some endpoint did eventually need to set it explicitly, it could just change what it passes to `setHref()`, but I believe we currently have no such endpoints and do not foresee ever having any.)
Test Plan:
- As a logged out user, clicked various links in Differential, Maniphest, Files, etc., always got redirected to a sensible place after login.
- Grepped for `setObjectURI()`, `getObjectURI()` (there are a few remaining callsites, but to a different method with the same name in Doorkeeper).
Reviewers: chad
Reviewed By: chad
Subscribers: hach-que
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14805
Summary:
Ref T10004. After a user logs in, we send them to the "next" URI cookie if there is one, but currently don't always do a very good job of selecting a "next" URI, especially if they tried to do something with a dialog before being asked to log in.
In particular, if a logged-out user clicks an action like "Edit Blocking Tasks" on a Maniphest task, the default behavior is to send them to the standalone page for that dialog after they log in. This can be pretty confusing.
See T2691 and D6416 for earlier efforts here. At that time, we added a mechanism to //manually// override the default behavior, and fixed the most common links. This worked, but I'd like to fix the //default// beahvior so we don't need to remember to `setObjectURI()` correctly all over the place.
ApplicationEditor has also introduced new cases which are more difficult to get right. While we could get them right by using the override and being careful about things, this also motivates fixing the default behavior.
Finally, we have better tools for fixing the default behavior now than we did in 2013.
Instead of using manual overrides, have JS include an "X-Phabricator-Via" header in Ajax requests. This is basically like a referrer header, and will contain the page the user's browser is on.
In essentially every case, this should be a very good place (and often the best place) to send them after login. For all pages currently using `setObjectURI()`, it should produce the same behavior by default.
I'll remove the `setObjectURI()` mechanism in the next diff.
Test Plan: Clicked various workflow actions while logged out, saw "next" get set to a reasonable value, was redirected to a sensible, non-confusing page after login (the page with whatever button I clicked on it).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14804
Summary:
Ref T9992. This is a step on the path to getting EditEngine working in Badges, Projects and Calendar.
This doesn't add a new `EditField` for icons yet, just standardizes the old stuff. New stuff is more general and I saved 150 lines of code.
I put the endpoint in Files because the similar "choose a profile picture" endpoint will definitely go there, and this endpoint might eventually feature, like, "draw your own icon~~" or something.
Test Plan:
- Created events, projects and badges with custom icons.
- Edited events, projects and badges, changing their icons.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9992
Differential Revision: https://secure.phabricator.com/D14799
Summary:
Ref T9908. Fixes T6205.
This is largely some refactoring to improve the code. The new structure is:
- Each EditField has zero or one "submit" (normal edit form) controls.
- Each EditField has zero or one "comment" (stacked actions) controls.
- If we want more than one in the future, we'd just add two fields.
- Each EditField can have multiple EditTypes which provide Conduit transactions.
- EditTypes are now lower-level and less involved on the Submit/Comment pathways.
Test Plan:
- Added and removed projects and subscribers.
- Changed task statuses.
- In two windows: added some subscribers in one, removed different ones in the other. The changes did not conflict.
- Applied changes via Conduit.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6205, T9908
Differential Revision: https://secure.phabricator.com/D14789
Summary: Ref T9927. Adds a "Blogs" section to PhameHome. Removes "New Post" Controller. Adds flipped layout for PHUITwoColumnView
Test Plan:
Test PhameHome, Ponder, New Post, etc. Mobile and Desktop states.
{F1022080}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: johnny-bit, Korvin
Maniphest Tasks: T9927
Differential Revision: https://secure.phabricator.com/D14744
Summary: If you archive a badge, remove it's presence in the main Phabricator UI. These are still accessible from `/badges/` for properity. Ref T9944
Test Plan: Archive a badge, weep uncontrollably.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9944
Differential Revision: https://secure.phabricator.com/D14730
Summary:
Fixes T9920. When hiding changes, tell users why so they can learn the comment rule (usually, "Changes from before your most recent comment are hidden."; sometimes they're hidden for pagination reasons).
Also use "Show Older Comments" instead of "Show older comments." for the action since I think it's a little more consistent to use title case for links/actions?
Test Plan:
- Viewed a task with a lot of comments, saw a "most recent comment" element.
- Artificially set page size to 3, saw a "lots of changes" hide.
- Grepped for removed string.
- Clicked both "show older stuff" links.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9920
Differential Revision: https://secure.phabricator.com/D14719
Summary: Ref T9908. This is the last of the things that need to swap over.
Test Plan:
- Created tasks from a workboard.
- Created tasks in different columns.
- Edited tasks.
- Used `?parent=..`.
- Verified that default edit form config now affects comment actions.
- No more weird comment thing on forms, at least for now.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14715
Summary: Ref T9908. This fixes "Create Subtask" so it works with the new stuff. Mostly straightforward.
Test Plan: Created some subtasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14706
Summary:
Ref T9908. This removes the "Create Another Empty Task", "Create A Similar Task" and "Create Another Subtask of Same Parent Task" workflow callouts on the task detail page, which are actions that show up after creating a task or creating a subtask.
- I think "Create Empty" isn't relevant now that we have "Create Task" nearby and the quick create menu?
- I'm not sure if "Create Similar" is worth keeping. If we do want to retain it, I'd maybe like to find a way to do it generically.
- I'm likewise not sure if "Create another subtask" is worth keeping.
Overall, these actions are weird/unusual and I'm not sure how valuable they are. I'm open to keeping "Similar" and/or "Subtask" but I'd like to verify that they're still valuable and make sure we have a reasonable design for them if we retain them.
For example, if we want to retain "Similar", maybe a better approach is just to add "Create Similar Object" to every action menu (which is now possible for EditEngine applications)? There's at least some interest in "Create Similar Repository" in Diffusion.
Also removes a very very old piece of "attached files" code.
Test Plan: Created and viewed some tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14705
Summary:
Fixes T8707. See that task for discussion. Browser behavior is apparently to ignore a newline immediately following a `<textarea>`, and ostensibly has been since the early 1800s.
This is the same fix Rails used when it encountered this issue in 2011, which gives me some confidence it is correct.
Test Plan:
- Edited a task with leading newlines in the description.
- Newlines were preserved correctly across multiple edits.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8707
Differential Revision: https://secure.phabricator.com/D14688
Summary: Provides more information that a post is a draft.
Test Plan:
Add a draft post, see new style. Check Blog as non-editor, don't see draft post.
{F1008655}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14613
Summary: Creates a new PhameBlogView which is more of a blog landing page with the latest posts. Management has moved to PhameManageController with a new timeline.
Test Plan:
Edit Blog, Publish, Subscribe, view posts.
{F1008400}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14608
Summary: This makes document views a little more automatic, and a little more style to the page. The Document itself remains on a pure white centered background, but footer and preceeding objects go back to the original body color. This provides a bit more depth and separation over content and definitions/comments.
Test Plan:
Tested Phriction, Diviner, Legalpad, Phame, Email Commands, HTTP Commands, with and without a footer.
{F1005853}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14582
Summary: New icons
Test Plan: Use new icons
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14568
Summary: Reuse PHUIMarkupPreviewView in Phame for consistency, less custom code. Also, doesn't work (JS issue).
Test Plan: New Post, Edit Post, Save Post
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14552
Summary:
Addresses T9814. Adds SVG files to Celerity maps. Adds a mask-icon.svg file that
I made by pulling the existing favicon into Illustrator and running trace on it.
This hardcodes the header color from the default theme, and doesn't pay attention
to customizations of the header.
Test Plan: I pinned the tab in Safari.
Reviewers: epriestley, #blessed_reviewers, chad
Reviewed By: #blessed_reviewers, chad
Subscribers: chad, Korvin
Maniphest Tasks: T9814
Differential Revision: https://secure.phabricator.com/D14527
Summary:
Ref T9132. Ref T4768. This is a rough v0 of ApplicationEditor, which replaces the edit workflow in Paste.
This mostly looks and works like ApplicationSearch, and is heavily modeled on it.
Roughly, we define a set of editable fields and the ApplicationEditor stuff builds everything else.
This has no functional changes, except:
- I removed "Fork Paste" since I don't think it's particularly useful now that pastes are editable. We could restore it if users miss it.
- Subscribers are now editable.
- Form field order is a little goofy (this will be fixed in a future diff).
- Subscribers and projects are now race-resistant.
The race-resistance works like this: instead of submitting just the new value ("subscribers=apple, dog") and doing a set operation ("set subscribers = apple, dog"), we submit the old and new values ("original=apple" + "new=apple, dog") then apply the user's changes as an add + remove ("add=dog", "remove=<none>"). This means that two users who do "Edit Paste" at around the same time and each add or remove a couple of subscribers won't overwrite each other, unless they actually add or remove the exact same subscribers (in which case their edits legitimately conflict). Previously, the last user to save would win, and whatever was in their field would overwrite the prior state, potentially losing the first user's edits.
Test Plan:
- Created pastes.
- Created pastes via API.
- Edited pastes.
- Edited every field.
- Opened a paste in two windows and did project/subscriber edits in each, saved in arbitrary order, had edits respected.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4768, T9132
Differential Revision: https://secure.phabricator.com/D14390
Summary:
Fixes T5752. This obsoletes a bunch of old patterns and I'll follow up on those with a big "go do a bunch of mechanical code changes" task. Major goals are:
- Don't load named queries multiple times on search pages.
- Don't require extra code to get standard navigation right on mobile.
- Reduce the amount of boilerplate in ListControllers.
- Reduce the amount of boilerplate around navigation/menus in all controllers.
Specifically, here's what this does:
- The StandardPage is now a smarter/more structured object with `setNavigation()` and `setCrumbs()` methods. More rendering decisions are delayed until the last possible moment.
- It uses this to automatically add crumb actions to the application menu.
- It uses this to automatically reuse one SearchEngine instead of running queries multiple times.
- The new preferred way to build responses is `$this->newPage()` (like `$this->newDialog()`), which has structured methods for adding stuff (`setTitle()`, etc).
- SearchEngine exposes a new convenience method so you don't have to do all the controller delegation stuff.
- Building menus is generally simpler.
Test Plan:
- Tested paste list, view, edit, comment, raw controllers for functionality, mobile menu, crumbs, navigation menu.
- Edited saved queries.
- Tested Differential, Maniphest (no changes).
- Verified the paste pages don't run any duplicate NamedQuery queries.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5752
Differential Revision: https://secure.phabricator.com/D14382
