[Notifier][WebProfilerBundle][FrameworkBundle] Add notifier section to profiler #36479

jschaedl · 2020-04-17T15:29:46Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	-
License	MIT
Doc PR	-

This is the first iteration of adding a profiler panel for the new Notifier component:

WebProfiler Toolbar:

WebProfiler Notifier Panel:

An example project can to test the new profiler panel can be found here: https://github.com/jschaedl/notifier-profiler-integration

jschaedl · 2020-04-17T15:30:18Z

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/notifier.html.twig

+    {% set events = collector.events %}
+
+    <span class="label {{ events.messages|length ? '' : 'disabled' }}">
+        <span class="icon">{{ include('@WebProfiler/Icon/mailer.svg') }}</span>


exchange icon

@javiereguiluz Can you work on an icon for the notifier profiler?

Sorry for being late, but our designer just sent us the icon for the notifier. It uses the "bell icon" which is universal for notifications:

<svg xmlns="http://www.w3.org/2000/svg" height="24" width="24" viewBox="0 0 24 24"><path fill="#AAA" d="M11.23,4.8c-3,.83-5.07,3.18-5.07,5.9H7.39c0-2.21,1.77-4,4.17-4.72ZM7.49,0A12.22,12.22,0,0,0,0,11.59H2.07A10.14,10.14,0,0,1,8.23,2Zm8.24,2a10.14,10.14,0,0,1,6.16,9.64H24A12.24,12.24,0,0,0,16.47,0ZM4.41,15.64V10.7a7.57,7.57,0,0,1,15.14,0v4.94l3.3,4.4H1.11Zm4.45,5.3A3.06,3.06,0,0,0,11.92,24H12a3.07,3.07,0,0,0,3.07-3.06Z"/></svg>

@javiereguiluz as this PR is already merged, it might be better if you create a new PR with the new icon instead.

Thank you @javiereguiluz I created a new PR: #38545

fabpot · 2020-08-06T04:04:12Z

@jschaedl Will you have time to finish this PR? Any blockers?

fabpot · 2020-08-13T11:05:09Z

We might need to take into account the new SentMessage class (not sure if this is worth it though).

jschaedl · 2020-08-17T06:24:17Z

@fabpot

@jschaedl Will you have time to finish this PR? Any blockers?

Sorry for the delay. There are no blockers so far, only limited time. But will try to work on this PR at the end of this week.

fabpot · 2020-09-30T08:03:26Z

@jschaedl Do you think you will have something that we can merge before the end of the week (for 5.2)? Even if it's not finished?

jschaedl · 2020-10-01T06:40:48Z

Hi @fabpot, yes I think so. I was planning to work on it tmrw again and will let you know how far I've come.

jschaedl · 2020-10-02T09:08:35Z

@fabpot Ready for review. :-)

jschaedl · 2020-10-02T09:09:30Z

src/Symfony/Component/Notifier/Transport/NullTransport.php

@@ -34,6 +34,10 @@ public function __construct(EventDispatcherInterface $dispatcher = null)

    public function send(MessageInterface $message): SentMessage
    {
+        if (null === $message->getTransport()) {
+            $message->transport((string) $this);


I needed to make sure that every transport has a name to show this name in the profiler panel.

fabpot · 2020-10-04T07:42:44Z

src/Symfony/Component/Notifier/Message/MessageInterface.php

@@ -25,4 +25,6 @@ public function getSubject(): string;
    public function getOptions(): ?MessageOptionsInterface;

    public function getTransport(): ?string;
+
+    public function transport(string $transport): self;


Not a big fan, but I don't have any better idea for now.

Maybe we could decorate the message with a DebugMessage that exposes this information?

I like this idea and gave it a try: #38474 WDYT?

…o profiler

fabpot · 2020-10-04T07:43:48Z

Thank you @jschaedl.

…tter in MessageInterface (jschaedl) This PR was merged into the 5.x branch. Discussion ---------- [Notifier] Introduce NullMessage and remove transport setter in MessageInterface | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no  | Deprecations? | no  | Tickets | -  | License | MIT | Doc PR | -  Follow-up PR of #36479 Commits ------- 5701e89 Introduce NullMessage and remove transport setter in MessageInterface

…l icon (jschaedl) This PR was squashed before being merged into the 5.x branch. Discussion ---------- [Notifier][WebProfilerBundle] Add notifier profiler panel icon | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no  | Deprecations? | no  | Tickets | | License | MIT | Doc PR | symfony/symfony-docs#...   relates to #36479 Commits ------- 303096e [Notifier][WebProfilerBundle] Add notifier profiler panel icon

carsonbot added Status: Needs Review WebProfilerBundle FrameworkBundle Feature labels Apr 17, 2020

jschaedl commented Apr 17, 2020

View reviewed changes

nicolas-grekas added this to the next milestone Apr 18, 2020

fabpot added the Notifier label Apr 21, 2020

jschaedl mentioned this pull request Sep 5, 2020

[Notifier] DataCollector for Notifier component not available #38009

Closed

jschaedl force-pushed the notifier-profiler-integration branch from c87bf07 to 77a07d9 Compare September 13, 2020 12:07

jschaedl marked this pull request as ready for review September 13, 2020 12:51

jschaedl force-pushed the notifier-profiler-integration branch from 77a07d9 to 3af0d99 Compare September 13, 2020 15:42

jschaedl force-pushed the notifier-profiler-integration branch 2 times, most recently from 7ecc8f9 to 45e23a7 Compare October 2, 2020 08:38

jschaedl commented Oct 2, 2020

View reviewed changes

fabpot reviewed Oct 4, 2020

View reviewed changes

fabpot approved these changes Oct 4, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Oct 4, 2020

[Notifier][WebProfilerBundle][FrameworkBundle] Add notifier section t…

f39e74b

…o profiler

fabpot force-pushed the notifier-profiler-integration branch from 45e23a7 to f39e74b Compare October 4, 2020 07:43

fabpot merged commit edb5fed into symfony:master Oct 4, 2020

nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020

fabpot mentioned this pull request Oct 5, 2020

Release v5.2.0-BETA1 #38417

Merged

jschaedl mentioned this pull request Oct 7, 2020

[Notifier] Introduce NullMessage and remove transport setter in MessageInterface #38474

Merged

jschaedl mentioned this pull request Oct 13, 2020

[Notifier][WebProfilerBundle] Add notifier profiler panel icon #38545

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Notifier][WebProfilerBundle][FrameworkBundle] Add notifier section to profiler #36479

[Notifier][WebProfilerBundle][FrameworkBundle] Add notifier section to profiler #36479

jschaedl commented Apr 17, 2020 •

edited

Loading

jschaedl Apr 17, 2020

fabpot Oct 2, 2020

javiereguiluz Oct 13, 2020

stof Oct 13, 2020

jschaedl Oct 13, 2020

fabpot commented Aug 6, 2020

fabpot commented Aug 13, 2020

jschaedl commented Aug 17, 2020

fabpot commented Sep 30, 2020

jschaedl commented Oct 1, 2020

jschaedl commented Oct 2, 2020

jschaedl Oct 2, 2020

fabpot Oct 4, 2020

xabbuh Oct 5, 2020

jschaedl Oct 7, 2020

fabpot commented Oct 4, 2020

[Notifier][WebProfilerBundle][FrameworkBundle] Add notifier section to profiler #36479

[Notifier][WebProfilerBundle][FrameworkBundle] Add notifier section to profiler #36479

Conversation

jschaedl commented Apr 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Aug 6, 2020

fabpot commented Aug 13, 2020

jschaedl commented Aug 17, 2020

fabpot commented Sep 30, 2020

jschaedl commented Oct 1, 2020

jschaedl commented Oct 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Oct 4, 2020

jschaedl commented Apr 17, 2020 •

edited

Loading