Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Widgets API for plugins #7861

Closed
tsteur opened this Issue May 8, 2015 · 2 comments

Comments

Projects
None yet
2 participants
@tsteur
Copy link
Member

tsteur commented May 8, 2015

While working on #7822 I noticed that it makes sense to change the Widgets API.

This is how we currently define a Widget: http://developer.piwik.org/guides/widgets
It was one of the first APIs that we revised and it works fine and is easy to use. Though it makes sense to refactor it with some features in mind etc. (#7822, #7131, #4734).

  • I'm pretty sure we will be able to maintain BC, we could break the API for Piwik 3.0.0
  • We possibly will be able to convert code from old logic to new logic automatically
  • Reports share a lot of similarities with Widgets (name, category, module, action, render, ...) and Reports are more or less widgets, they could implement the same interface or extend each other or ...
  • The code generator will allow us to generate specific widgets (this is currently not the case as we'd have to manipulate an existing widget class)
  • We will be later able to add widgets to a page in the UI (if wanted)
  • If there are many dependencies because there are many different widgets we will have less dependencies when having one class per widget
  • Types/Properties (refs #7131 and #4734) will be able to rename widgets, to remove them, etc.
  • We could later allow plugins to overwrite existing widgets (not important, just saying would be possible)
  • We can give widgets an order
  • We can generate translation keys automatically
  • We might have in soon another "thing" beside Reports and Widgets related to ReportsByDimension (ignore it for now)

The API itself will look similar to a Report I reckon.

  • We will have all widgets in a directory Widgets
  • A widget will be named by the action similar to reports eg Widgets\GetDonateForm.php

TODO:

  • What about Widgets::configureWidgetsList(), shall we call this in each Widget itself?

@tsteur tsteur self-assigned this May 8, 2015

@tsteur tsteur added this to the 2.14.0 milestone May 8, 2015

tsteur added a commit that referenced this issue May 8, 2015

tsteur added a commit that referenced this issue May 11, 2015

@tsteur

This comment has been minimized.

Copy link
Member Author

tsteur commented May 11, 2015

I started working on new WIdget API see new Widget base class https://github.com/piwik/piwik/compare/7861#diff-2627bb10f1c9060b562ef684be844e8dR1 and an example widget https://github.com/piwik/piwik/compare/7861#diff-328e5dc766c1f330bc3efc090ee7d32fR21

Fow now it is straight forward and very similar to the Report class.

There are currently some "problems":

  • configureWidgetList(WidgetList $widgetlist) does not really belong there (does two things, creates a new widget, and configures other widgets). Example: https://github.com/piwik/piwik/blob/0ae422cf0389e6a0c1a663340ebcaee77d177852/plugins/Goals/Widgets/WidgetConfig.php#L15 (ignore the isEnabled) The configureWidgetList is meant to add multiple widgets for one class (eg for each goal) and / or to remove widgets from the list. We have 2 more alternatives, none of them is really great:
    • Static configureWidgetsList method (would be much better I think)
    • Use an event Piwik::postEvent('WidgetsList.addWidgets/filterWidgets', array($widgetsList))
    • Let plugin developers define a class WidgetsList or so in their plugin which will allow them to configure it.
    • I'm sure there are more possible solution
  • If widgets require dependencies we will have to create all of them which can slow down page creation. It's similar to commands just that for commands performance is not such a big deal etc. Also of course other problems come with having to create dependencies similar to commands. We could use a similar concept as suggested in #7175 for example. Just that we will need dependencies in that static config method for example for such cases https://github.com/piwik/piwik/compare/7861#diff-13dfb9f63e96dfcbc0c5b90973b0a55bR19 . Having such a config might be beneficial for Report classes as well and especially when working on #7131 and #4734 where a eg a type "MobileApp" can change the name of a report/widget and enable/disable it.
  • Having a WidgetConfiguration class (see previous item re dependencies) could also help to create a widget for a report like this https://github.com/piwik/piwik/compare/7861?expand=1#diff-04ac2e4c6abd747be0f7894ca9bede54R25
  • We have similar logic for Reports, and in the future maybe more such things (eg ReportByDimensionView). Eg both have a render method meaning they could implement something like Renderable and they have methods like getAction, getMethod, checkEnabled which could implement something like Routable. As Piwik 3.0 is coming closer we have the chance to change a few things but I don't like to use interfaces here as it will be problematic to keep BC.

If any ideas or more feedback @mattab @diosmosis @mnapoli please comment. As more problems come up I will comment as well.

Maybe something like this:

class GetVisitorProfilePopup extends \Piwik\Plugin\Widget
{
    public static function configure(WidgetConfig $config)
    {
        $config->setCategory('Live!');
        $config->setName('Live_VisitorProfile');
        $config->setOrder(25);
        $config->disable();
    }

    public function render()
    {
    }

    public static function configureWidgetsList(WidgetsList $widgetsList)
    {
        // only needed in rare cases when removing a widget or so
    }

}

WidgetConfig can be also used in a WidgetsList.addWidgets event (although this is currently deprecated)

$config = new WidgetConfig();
$config->setCategory('Live!');
$config->setName(Piwik::translate('UserCountryMap_RealTimeMap'));
$config->setModule('UserCountryMap');
$config->setAction('realtimeMap');
$config->setOrder(5);
WidgetsList::getInstance()->addWidget($config);

// instead of 
WidgetsList::add('Live!', Piwik::translate('UserCountryMap_RealTimeMap'), 'UserCountryMap', 'realtimeMap', array(), $order = 5);

And it will be probably used in reports to create a widget from a report eg

    protected function init()
    {
        parent::init();
        $this->order = 2;

        $this->createWidget()
             ->setName('Live_VisitorLog')
             ->setOrder(10)
             ->setParameters(array('small' => 1));
    }
@tsteur

This comment has been minimized.

Copy link
Member Author

tsteur commented Aug 20, 2015

fixed via #8442

@tsteur tsteur closed this Aug 20, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.