Page MenuHomePhabricator
Differential D18121

Allow dashboard panels to be found by monogram
ClosedPublic
Actions

Authored by 20after4 on Jun 13 2017, 9:43 PM.
Tags
None
Referenced Files
F13038056: D18121.id43603.diff
Mon, Apr 15, 10:29 PM
Unknown Object (File)
Sat, Apr 13, 10:52 PM
Unknown Object (File)
Sat, Apr 13, 8:40 PM
Unknown Object (File)
Sat, Apr 13, 7:24 PM
Unknown Object (File)
Sat, Apr 13, 5:26 PM
Unknown Object (File)
Sun, Apr 7, 5:48 AM
Unknown Object (File)
Sun, Apr 7, 5:36 AM
Unknown Object (File)
Sun, Mar 31, 11:37 PM
View All 89 Files
Subscribers
Korvin

Details

Reviewers
chad
epriestley
Group Reviewers
Blessed Reviewers
Commits
rPe1850b3c4e4e: Allow dashboard panels to be found by monogram
Summary

Just add the monogram to the datasource's name field
so that it will match when typing Wnn in the typeahead field.

Test Plan

Tested locally on my dev phab. Try searching for a panel
by monogram in the 'Add existing panel' dialog on the
'arrange workboard' interface.

Previously: typing W123 showed no results.

After this change: typing W123 finds the panel W123

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

20after4 created this revision.Jun 13 2017, 9:43 PM
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptJun 13 2017, 9:43 PM
Herald added a subscriber: Korvin. · View Herald Transcript
Harbormaster completed remote builds in B17498: Diff 43602.Jun 13 2017, 9:43 PM
Harbormaster completed remote builds in B17498: Diff 43602.
20after4 added a comment.EditedJun 13 2017, 9:44 PM

This seems like it was probably just an oversight and it's a trivial change so I'm upstreaming. Already in queue to be deployed downstream in Wikimedia's install.

epriestley accepted this revision.Jun 13 2017, 9:45 PM

I think you can just remove setDisplayName(), it should default to the name if not explicitly provided.

This revision is now accepted and ready to land.Jun 13 2017, 9:45 PM
epriestley added a comment.Jun 13 2017, 9:47 PM

And, yeah, I can't think of any reason we'd want the behavior as written before this change. It looks like it came from D17348 but I imagine the intent was just this change, not "show the monogram but don't let users select with it".

Closed by commit rPe1850b3c4e4e: Allow dashboard panels to be found by monogram (authored by 20after4, committed by 20after4). · Explain WhyJun 13 2017, 9:48 PM
This revision was automatically updated to reflect the committed changes.
20after4 added a comment.Jun 13 2017, 9:49 PM

Indeed, I tested this and removing the setDisplayName retains the correct behavior (show the monogram AND let you search by monogram)

chad edited edge metadata.Jun 13 2017, 10:15 PM

I probably meant “don’t show, but let users search with it"

Revision Contents
Changeset List

PathSize
src/
applications/
dashboard/
typeahead/
3 lines

Diff 43603

View Options

src/applications/dashboard/typeahead/PhabricatorDashboardPanelDatasource.php

Loading...
Phabricator · Built by Phacility