Use plugins for collectors, create Data helper to simplify Toolbar

magento-hackathon / magento2-toolbar

Toolbar with developer and merchant functionality

Open Software License 3.0

28 stars 7 forks source link

Use plugins for collectors, create Data helper to simplify Toolbar #11

Closed barryvdh closed 8 years ago

barryvdh commented 8 years ago

Moved the default collectors to the di.xml configuration/plugins.
Created Helper class with some convenience acces to request/url/state/config
Simplify the Toolbar class by leveraging the Helper class.

Todo;

Actually create config

/cc @elzekool

barryvdh commented 8 years ago

So I guess this would be very basic, but what kind of config do we need?

Enabled (yes/no/auto (based on dev mode))
Restrict per IP
Collect only, show toolbar only for IP or admin user? ..

And we should set the default for the collectors, eg. check certain config before adding collectors/listening to all events. And all collectors should be able to disable/enable on the same way.

barryvdh commented 8 years ago

Hmm, I can't really get the URL generation to work properly. The admin part is messing it up a bit, because it adds some querystring after the url. I tried to use the storemanager, but that adds adminhtml to the url. The current way is working on developer mode, unless I run static di compile :/

Any ideas? I just want to baseUrl without any localisation/etc. Should I just get it from the config or something?

elzekool commented 8 years ago

Hi @barryvdh,

Excuse for the late reply, was a busy weekend ;-) My experience is, like most developers, very limited with Magento2. I have a look at it, 2 people see more then one.

barryvdh commented 8 years ago

I've added the UrlInterface to the Toolbar class + JavascriptRenderer, to make it clear whose has which dependancies.

This seems to work normally, except after setup:di:compile, then the area code is not set. I assume because we're injecting after the usual run (to get as much data as possible). There is an emulateState method, which we can use to pretend we're in frontend, which seems to be what we want.

The only thing I'm running in to is Recoverable Error: Object of class Magento\Framework\ObjectManager\ObjectManager could not be converted to string in ..., after using php bin/magento setup:di:compile, which happens because the optional $name parameter is filled in as a ObjectManager which obviously doesn't work. I'm guessing this is actually a Magento bug, or I'm missing something really stupid. (Clearing var/di makes it work again..) Maybe @jissereitsma knows this, as our DI specialist ;) Otherwise I'll report a bug in magento2

barryvdh commented 8 years ago

Okay I've worked around the issue by extending the MessageCollector and don't use an optional param. Also fixed the version which they removed from the AppInterface in 2.1.0-rc (hooray for SemVer :/)

You good with this? Then I'll merge.