Closed Bug 1596023 Opened 5 years ago Closed 5 years ago

Add a "New Changes" link to the Phabricator sidebar

Categories

(Conduit :: Phabricator, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: glob, Assigned: mhentges, Mentored)

References

(Blocks 2 open bugs)

Details

(Keywords: conduit-triaged, good-first-bug)

Attachments

(1 file)

Create an extension to add a link to the Phabricator sidebar to make it easier to identify changes since your last review.

The URL should append /new/ to the revision's URL. eg. https://phabricator.services.mozilla.com/D13125/new/

The link should be greyed out for unauthenticated page loads.

Keywords: conduit-triaged
Priority: -- → P2

This would be a good starter bug. We currently add the link to Lando to this sidebar, which is done with this code: https://github.com/mozilla-services/phabricator-extensions/blob/a7d33414a79c29c287c1547d6236b9fc824f2888/extensions/src/lando/events/LandoLinkEventListener.php#L10

This new link should use a similar mechanism.

Mentor: pzalewa
Keywords: good-first-bug

The link should be greyed out for unauthenticated page loads.

Should we also grey out the link if the current user hasn't performed an action on the differential?
The /new/ view didn't make sense to me until I tried it with the right context. I'm worried other users will click the link in the wrong context, and be confused when nothing happens. For example:

  • A reviewer clicks on an unreviewed differential with multiple changes. They see the "New Changes" link and click it, but the newly-loaded page doesn't look like it changed. 🤔

By disabling it in more situations and giving it hovertext ("you haven't operated on this differential yet" | "you need to be logged in"), we can guide users towards understanding how it's used.

If you agree with ^, I'll create a follow-up ticket.

Assignee: nobody → mhentges

(In reply to Mitchell Hentges [:mhentges] from comment #2)

If you agree with ^, I'll create a follow-up ticket.

Those sound like usability wins to me. Not sure how much effort/maintenance they'll require though, so filing them as a followup would be good.

Blocks: 1604841

A new event listener was added to put the "New Changes" link on the revision page.

This link is disabled for users who are already viewing "New Changes", as well as for unauthenticated users.
I'd prefer to add some hover-text for the link when it's disabled, but I couldn't immediately see how.
I'll leave that work for the follow up ticket 1604841.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: