Summary:
`strlen()` was used in Phabricator to check if a generic value is a non-empty string.
This behavior is deprecated since PHP 8.1. Phorge adopts `phutil_nonempty_string()` as a replacement.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If phutil_nonempty_string() throws an exception in your
instance, report it to Phorge to evaluate and fix that specific corner case.
Closes T15296
Test Plan: Applied this change and Home page rendered in web browser.
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15296
Differential Revision: https://we.phorge.it/D25145
Summary:
Fix numerous PHP 8.1 RuntimeExceptions caused by the deprecation of strlen(null).
The strlen() was used in Phabricator to check if a generic value was a non-empty string.
For this reason, Phorge adopts phutil_nonempty_string() that checks that.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If your phutil_nonempty_string() throws an exception, just
report it to Phorge to evaluate and fix together that specific corner case.
Closes T15264
Test Plan: Phorge homepage is displayed on PHP 8.1 after applying these changes
Reviewers: O1 Blessed Committers, valerio.bozzolan
Reviewed By: O1 Blessed Committers, valerio.bozzolan
Subscribers: speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno
Maniphest Tasks: T15264
Differential Revision: https://we.phorge.it/D25132
Summary:
Depends on D20373. Ref T13272. When you put Dashboards in Favorites, the render without a navigation menu, which is kind of weird.
Instead, make Favorites display a navigation menu. This effectively turns it into a weird cross between the home page and a portal, but so be it.
Also change the icon from "star" to "bookmark" since I think that a clearer hint about how it works.
Test Plan: Viewed dashboards in favorites, got a navigation menu. Edited items from portals, home, favorites.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13272
Differential Revision: https://secure.phabricator.com/D20374
Summary:
Depends on D20357. Ref T13275. Now that there's a stronger layer between "stuff in the database" and "stuff on the screen", these subclasses all need to emit intermediate objects instead of raw, HTML-producing view objects.
This update is mostly mechanical.
Test Plan:
- Viewed Home, Favorites, Portals, User Profiles, Project Profiles.
- Clicked each item on each menu/profile type.
- Added every (I think?) type of item to a menu and clicked them all.
- Grepped for obsolete symbols (`newNavigationMenuItems`, `willBuildNavigationItems`).
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13275
Differential Revision: https://secure.phabricator.com/D20358
Summary:
Depends on D20355. Ref T13275. Ref T13247. Currently, "Hamburger" menus are not automatically built from navigation menus. However, this is (I'm almost completely sure?) a reasonable and appropriate default behavior, and saves us some code around profile menus.
With this rule in place, we can remove `setApplicationMenu()` and `getApplicationMenu()` from `StandardPageView`, since they have no callers.
This also updates a lot of profile menu callsites to a new API which is added in the next change.
Test Plan: See the next two changes.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13275, T13247
Differential Revision: https://secure.phabricator.com/D20356
Summary:
Depends on D20353. Ref T13275. This is just some small quality-of-life fixes:
- When you add items to menus, they currently go below the "Edit Menu/Manage Menu" links by default. This isn't a very good place for them. Instead, lock "edit" items to the bottom of the menu.
- Lock profile pictures to the top of the menu. This just simplifies things a little.
- Show more iconography hints on the "edit menu items" UI.
- Add a "drag stuff to do things" hint if some stuff can be dragged.
Test Plan:
- Added new items to a Portal, they didn't go to the very bottom. Instead, they went above the "Edit/Manage" links; a sensible place for them.
- Viewed the "edit menu items" screen, saw more hints and visual richness.
- Viewed/edited Home, Projects, Portals, Favorites
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13275
Differential Revision: https://secure.phabricator.com/D20355
Summary:
Depends on D20352. Fixes T12949. If a user adds a link (for example, to a workboard) that takes you to the same page but with some URI parameters, we'd prefer to highlight the "link" item instead of the default "workboard" item when you click it.
For example, you add a `/query/assigned/` "link" item to a workboard, called "Click This To Show Tasks Assigned To Me On This Workboard", i.e. filter the current view.
This is a pretty reasonable thing to want to do. When you click it, we'd like to highlight that item to show that you've activated the "Assigned to Me" filter you added.
However, we currently highlight the thing actually serving the content, i.e. the "Workboard" item.
Instead:
- When picking what to highlight, look through all the items for one with a link to the current request URI.
- If we find one or more, pick the one that would be the default.
- Otherwise, pick the first one.
This means that you can have several items like "?a=1", "?a=2", etc., and we will highlight them correctly when you click them.
This actual patch has some questionable bits (see some discussion in T13275), but I'd like to wait for stronger motivation to refactor it more extensively.
Test Plan:
- On a portal, added a `?a=1` link. Saw it highlight properly when clikced.
- On a workboard, added a link to the board itself with a different filter. Saw it highlight appropriately when clicked.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T12949
Differential Revision: https://secure.phabricator.com/D20353
Summary:
Ref T13272. Currently, the hard-coded default homepage looks like a dashboard but is actually rendered completely manually.
This means that various panel rendering improvements we'd like to make (including better "Show More" behavior and better handling of overheated queries) won't work on the home page naturally: we'd have to make these changes twice, once for dashboards and once for the home page.
Instead, build the home page out of real panels. This turns out to be significantly simpler (I think the backend part of panels/dashboards is mostly on solid footing, the frontend just needs some work).
Test Plan:
Loaded the default home page, saw a thing which looked the same as the old thing. Changes I know about / expect:
- The headers for these panels are no longer linked, but they weren't colorized before so the links were hard to find. I plan to improve panel behavior for "find/more" in a followup.
- I've removed the "follow us on twitter" default NUX if feed is empty, since this seems like unnecessary incidental complexity.
- (Internal exception behavior should be better, now.)
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13272
Differential Revision: https://secure.phabricator.com/D20333
Summary: Adds a `MenuName` method to applications that `ProfileMenuItem` uses instead of the application name if set. This improves the home/menu/new user experience at little cost. Also renamed the label from Applications to Favorites, since this menu gets altered to provide more than just applications. This also allows instances to set back to Maniphest if they so choose. Overall I think this direction resolves 95% of my concerns, with maybe a small potential downside which I don't really anticipate. We already name Dashboard panels by their object, and that hasn't really caused confusion. I think these links are similar. I click 'Tasks' and get presented a list of my tasks from Maniphest.
Test Plan: Review each of the name changes as a default new install and a modified install.
Reviewers: epriestley, amckinley
Reviewed By: epriestley
Spies: Korvin
Differential Revision: https://secure.phabricator.com/D18524
Summary: We moved to having "no data" strings render in italics, but sometimes it doesn't make sense. This renders out the panel a little more expected.
Test Plan: Clean install of Phabricator, read home page activity box.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17415
Summary: Ref T12174. Drag-and-drop-to-upload requires some stuff in the document. Put that stuff on all the content pages (currently: dashboards, magic home), not just the builtin home.
Test Plan:
- Dragged-and-dropped onto a Home dashbboard to upload.
- Viewed, and dragged-and-dropped onto "builtin home" to upload.
- Dragged onto "Edit Menu" for home, no upload.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17307
Summary: Fixes T12187. Ref T12190. See T12190 for discussion of why this escaped notice.
Test Plan:
- Commented out the `error_reporting()` clause around file inclusion.
- Reproduced the error in PHP7.
- Corrected the method signature.
- Reloaded the page, no more error.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12190, T12187
Differential Revision: https://secure.phabricator.com/D17297
Summary: Ref T12174. This could be a little more verbose.
Test Plan: Review Global Menu Items
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17294
Summary:
Ref T12174.
- Go back to the old mobile behavior (full-screen menu by default, click to see content).
- Hide crumbs from all Home content UIs. I left them on the edit/configure UIs since they feel a little less out-of-place there and some have multiple levels.
Test Plan:
Viewed Home on mobile, viewed `/home/` on mobile.
Also, saw no crumbs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17290
Summary: Ref T12174. Fallback behavior on this already appears to be sensible.
Test Plan:
- Hid "Magic Home".
- Viewed homepage with no dashboards on the menu.
- Saw "Magic Home" content, with no item in the menu selected, which seems reasonable.
{F2557022}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17289
Summary:
Ref T12174. Setup is:
- Allow public access.
- Don't touch the default menu.
- Visit `/` while logged out.
Currently, you see "magic home" as content, but don't actually see the menu item.
Instead, show the menu item.
Test Plan: {F2557000}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17288
Summary: Ref T12174. We were always setting a name via builtins so the tooltip was always set. Fix the calls here.
Test Plan: Add "Badges", see tooltip, give "Badges" a name of "Badges", don't see tooltip.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17284
Summary: Ref T12174. These items could fatal (`$item not defined`) if the viewer was not logged in.
Test Plan: - Viewed home as a logged-out user.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17274
Summary:
Ref T12174. This fixes more bugs than it creates, I think:
- Dashboards now show the whole menu.
- Project and home items now show selected state correctly.
- The "choose global vs personal" thing is now part of MenuEngine, and the same code builds it for Home and Favorites.
- Home now handles defaults correctly, I think.
Maybe regression/bad/still buggy?:
- Mobile home is now whatever the default thing was, not the menu?
- Title for dashboard content or other items that render their own content is incorrectly always "Configure Menu" (this was preexisting).
Test Plan:
- Created, edited, reordered, disabled, deleted and pinned personal and global items on home, favorites, and projects.
- Also checked User profiles.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12174
Differential Revision: https://secure.phabricator.com/D17273
Summary: Ref T11957. Needs some more polish, but I think everything here is square.
Test Plan: Add personal/global items to home, test mobile. Test workboards / colors.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: 20after4, rfreebern, Korvin
Maniphest Tasks: T11957
Differential Revision: https://secure.phabricator.com/D17259
Summary:
Ref T11957. When you click a dashboard item, it now sends you to `/<app>/item/view/123/`, which renders the proper crumbs, navigation, etc., with the dashboard as page content.
This works as you'd expect in Projects:
{F2508568}
It's sliiiightly odd in Favorites since we nuke the nav menu, but seems basically fine?
{F2508571}
Test Plan:
- Created a dashboard panel on a project.
- Clicked it, saw it render.
- Made it the default panel, viewed project default screen, saw dashboard.
- Disabled every panel I could, still saw reasonable behavior (this is silly anyway).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11957
Differential Revision: https://secure.phabricator.com/D17255
Summary: Ref T11957, just lays in some minor bug fixes. Sets correct menu, removes sidebar on edit.
Test Plan: Test /menu/ on home with Admin and Normal accounts.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11957
Differential Revision: https://secure.phabricator.com/D17247
Summary: Ref T12136. This just yanks the band-aid off. Fundamentally these were useful well before Dashboards and advanced bucketing, but not so much any more. They also have some performance hit.
Test Plan: Add some tasks and diffs onto a new instance, see there is no count on the home menu bar.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12136
Differential Revision: https://secure.phabricator.com/D17238
Summary:
Ref T12140. The major effect of this change is that uninstalling "Home" (as we do on admin.phacility.com) no longer uninstalls the user menu (which is required to access settings or log out).
This also simplifies the code a bit, by consolidating how menus are built into MenuBarExtensions instead of some in Applications and some in Extensions.
Test Plan:
- While logged in and logged out, saw main menus in the correct order.
- Uninstalled Favorites, saw the menu vanish.
- Uninstalled Home, still had a user menu.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12140
Differential Revision: https://secure.phabricator.com/D17239
Summary: Builds out more UI to reinforce just who you are in this world... A perfect person.
Test Plan:
Look at myself a lot.
{F2435202}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D17224
Summary: Ref T5867. I sure love Javascript.
Test Plan: Navigated between Home, Diffusion and Differential, opening the user profile menu. Saw appropraite help items.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5867
Differential Revision: https://secure.phabricator.com/D17214
Summary:
Still lots to fix here, punting up since I'm running into a few roadblocks.
TODO:
[] Sort Personal/Global correctly
[] Quicksand in Help Items correctly on page changes
Test Plan: Verify new menus work on desktop, tablet, mobile. Test logged in menus, logged out menus. Logging out via a menu, verify each link works as expected. Help menus get build when using an app like Maniphest, Differential. Check that search works, preferences still save.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T12107
Differential Revision: https://secure.phabricator.com/D17209
Summary: Ref T11957, builds out `/home/menu/` as a basic structure for adding/editing the homepage menu.
Test Plan: visit `/home/menu/` and add items to global and personal. Not wired to anything.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11957
Differential Revision: https://secure.phabricator.com/D17180
Summary: Creates a background that renders inside the Quicksand frame, through sorcery.
Test Plan: Turn on Quicksand, visit lots of pages. See correct background colors. This probably blows something up I'm not testing.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16642
Summary: Ref T11132, swaps in new UI for welcome page using guide modules
Test Plan: Test instance and non instance guides. Test each setting. Unclear on how to test people / Phacility. Just change the URL link?
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11132
Differential Revision: https://secure.phabricator.com/D16482
Summary: Ref T11132. Adds a text panel to feed if no stories are present and the user is an admin. Seems ok-ish for 15 minutes. Happy to take content suggestions.
Test Plan: Make a new install, see panel. Log in as new user, don't see panel.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11132
Differential Revision: https://secure.phabricator.com/D16479
Summary: Ref T11132. This is a new default default (no dashboard) homepage. It offers (Diffs) (Tasks) (Repositories) in the main column and (Feed) in the side column. No NUX stuff, No logged out public view (upcoming diff). This should be complete, but unclear how to bucketize Differential.
Test Plan: Test new account's default homepage.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T11132
Differential Revision: https://secure.phabricator.com/D16449
Summary: Makes sidenav disappear again on projects/profiles, but shows it on home again (tablet views).
Test Plan: Visit Profile/Projects/Home on mobile, desktop, and tablet. See nav disappear correctly.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D16369
Summary:
Ref T4103. Two small improvements:
- Don't work as hard to validate translations. We just need to know if a translation exists, we don't need to count how many strings it has and build the entire menu.
- Allow `getUserSetting()` to work on any setting without doing all the application/visibility checks. It's OK for code to look at, say, your "Conpherence Notifications" setting even if that application is not installed for you.
Test Plan: Used XHProf and saw 404 page drop from ~60ms to ~40ms locally.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16046
Summary:
Ref T10077. Currently, we issue 6+ queries on every page to build this menu, since the menu is built application-by-application.
Build the menu with dedicated modules instead so a single "EditEngine" module can provide all of them with one query.
I'd like to reduce this to 0 queries but I'm not totally sure what we want to do with this menu.
This change removes these items, because EditEngine can not currently provide them:
- Calendar: Eventually via EditEngine eventually.
- Conpherence: Probably via EditEngine, doesn't seem too important.
- People: Maybe via EditEngine, doesn't seem too important? "Welcome" is likely better?
- Pholio: Eventually via EditEngine.
It adds a bunch of other items as a side effect:
{F1677151}
This reduces the queries issued on every page by ~5.
This also makes quick create actions visible while logged out (see T7073).
Test Plan:
- Viewed menu while logged in.
- Viewed menu while logged out.
- Viewed standalone version of menu.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10077
Differential Revision: https://secure.phabricator.com/D16045
Summary:
Ref T4103. A few bits here:
- We have an ancient "tiles" preference which was just a fallback from 2-3 years ago. Throw that away.
- Modenize the other pinned stuff. We should likely revisit this after the next homepage update but I just left the actual defaults alone for now.
- Lightly prepare for global default editing.
- Add a "reset to defaults" option.
Test Plan:
- Pinned, unpinned, reordered and reset application homepage order.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4103
Differential Revision: https://secure.phabricator.com/D16028
Summary:
Ref T10939. If you haven't installed a dashboard, we show an "Active Revisions" panel on the homepage by default. I waited a bit to update this, but the new buckets don't seem to have caused any major problems so far.
Update this to use the new logic. I'm just showing "must review" + "should review", which is similar to the old beahvior.
Also replace the notification count with this same number. This is a little different from the old behavior, but simpler, and I think we should probably move toward getting rid of these counts completely.
Test Plan:
- Viewed homepage as logged-in user, saw my revisions (including revisions I have authority over only because of project membership).
- Saw consistent notification count.
- Grepped for removed method.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10939
Differential Revision: https://secure.phabricator.com/D15950
Summary: Gives a bit more flexibility to add anything to the right side of PHUIHeaderView.
Test Plan: Test Maniphest, Workboards, Project Home, Differential. Grep for `addActionIcon` use. Fixes T10518
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Maniphest Tasks: T10518
Differential Revision: https://secure.phabricator.com/D15402
Summary: Mostly for consistency, we're not using other forms of icons and this makes all classes that use an icon call it in the same way.
Test Plan: tested uiexamples, lots of other random pages.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15125
Summary:
Ref T10144. This isn't comprehensive, but we can give it a try and see how it feels?
- EditEngine forms now say "Tags" instead of "Projects".
- Modern SearchEngine forms now say "Tags" instead of "Projects".
- For clarity, replaced as much "in project" language as I could find with "tagged with project" language.
Test Plan: reading / grepping + used "not tagged with any project" token
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10144
Differential Revision: https://secure.phabricator.com/D15108
Summary:
Ref T9908. We can get a double-header with this ("Quick Create", "Create Task") which looks weird.
The behavior of this menu is probably obvious enough on its own from context and the "+" icon.
Test Plan: Opened menu, no more "Quick Create" item.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14696
Summary:
Ref T9908. When there are custom / renamed / policy considerations for applications, respect them in the quick create menu.
This has some performance implications, in that it makes every page slower by two queries (and potentially more, soon), which is quite bad. I have some ideas to mitigate this, but it's not the end of the world to eat these queries for now.
Test Plan: {F1017316}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14693
Summary: Ref T9690. I wanted to do an example of how to do these but it looks like most of them are trivial (no callsites) and the rest are a little tricky (weird interaction with frames, or in Releeph).
Test Plan:
- Used `grep` to look for callsites.
- Hit all applications locally, everything worked.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9690
Differential Revision: https://secure.phabricator.com/D14385
Summary:
Fixes T9279. Modernizes the SearchEngine and Query classes. User-facing changes:
- Added order by commit date, default to order by commit date with newest commits first.
- Added explicit "Needs Audit by".
- Added new `packages(...)` typeahead function.
- Picked up automatic subscribers, projects, and order fields.
This changes behavior a little bit: we previously attempted to exclude, e.g., commits which a package you own needs to audit, but which you have resigned from. This is difficult in general and I think it needs a more comprehensive solution. This shouldn't impact users much, anyway.
Test Plan: {F767628}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9279
Differential Revision: https://secure.phabricator.com/D14013
