Fixes T5575. Moves "All" links into title/header. Mark all read floats left, and connection status sits in footer. Also added hints to enable notifications (it's a cool feature).
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T5575: Connection info in Notifications header needs rethinking
- Commits
- Restricted Diffusion Commit
rPdc69c4e58c2a: Touch up notification/messages panels
Tested locally both menus.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- notification-ui
- Lint
Lint Warnings Severity Location Code Message Warning src/applications/notification/controller/PhabricatorNotificationPanelController.php:55 TXT3 Line Too Long - Unit
No Test Coverage - Build Status
Buildable 2232 Build 2236: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
I could also see not showing the 'not enabled'. Unclear how many installs even know its an option.
Comment Actions
I'd favor leaving the hint hidden for now -- it's still pretty hard to set up and debug (T5732, T5366, T5365, no version checking for node version has bitten a couple of users). It's better than it used to be, but the major reason I'm not pushing it harder is that there are still a bunch of common issues that we haven't smoothed out yet.
It's not clear to me that "Notifications" is a link.
| src/applications/notification/controller/PhabricatorNotificationPanelController.php | ||
|---|---|---|
| 55 | Use PhabricatorEnv::getDoclink() | |
Comment Actions
I erred on the side of clean ui with no "obvious" link. I think people will figure it out, but will put it back if not.