View Issue Details

IDProjectCategoryView StatusLast Update
11704Bug reports[All Projects] Pluginspublic2016-10-20 15:16
ReporterYuriyBabenko Assigned Toollehar  
PrioritynoneSeverityminor 
Status feedbackResolutionopen 
Product Version2.52.x 
Target VersionFixed in Version 
Summary11704: No implementation of beforeSurveyBarRender event
Description

The plugin events page (https://manual.limesurvey.org/Plugin_events) lists the 'beforeSurveyBarRender' event. On my copy of 2.52, there are no references to this string anywhere in the codebase, and my plugin's implementation of this event does not get called.

Steps To Reproduce

grep -ri "beforeSurveyBarRender" *;

TagsNo tags attached.
Complete LimeSurvey version number (& build)20160922
I will donate to the project if issue is resolvedNo
Browser
Database & DB-Version260
Server OS (if known)OSX
Webserver software & version (if known)Apache 2.x
PHP Version5.6.x

Activities

DenisChenu

DenisChenu

2016-09-23 08:51

developer   ~40925

Else : Thanks for the update of the manual :)

There are some event i didn't know and don't found a good way to use it :)

What doi you think of purpose of beforeSurveyBarRender ?

@olle : i assign this to you. It's not a plugin event inside another plugin ? Think it's when you work on "Menu"

YuriyBabenko

YuriyBabenko

2016-09-23 09:08

reporter  

survey bar menu.png (22,502 bytes)
survey bar menu.png (22,502 bytes)
YuriyBabenko

YuriyBabenko

2016-09-23 09:11

reporter   ~40926

@DenisChenu - I've attached an image of what I believe to be the Survey Bar Menu. My original intent was looking for a way to add items to this menu. Ideally, I would like my plugin to have full control over the items in this menu (and other menus, but that's another topic).

The Survey Bar Menu's structure is built in the _surveybar() method in Survey_Common_Action.php. I believe the event should be triggered from that method. What I would like to see is that method creating a default set of menu links, and then passing that data to the event - thereby allowing plugins to change/add the menu links - and then expose the data to the surveybar_view.php file that would subsequently loop through the menu items and render them.

If this issue is still unresolved by tomorrow and there are no objections, I'll build out the above functionality and issue a Pull Request on GitHub.

Cheers.

DenisChenu

DenisChenu

2016-09-23 09:32

developer   ~40927

Last edited: 2016-09-23 09:32

View 2 revisions

Hi YuriyBabenko : you'r totally right : we must have a way to extend all menu.

BUT : i don't think we need an event for each menu ;)
Think it's best to have on beforeMenu event with

  • object type + object if (for example survey + surveyId)
  • menu name/type
    And return
  • menuItem object

Actually : i think this 2 event :
https://manual.limesurvey.org/BeforeAdminMenuRender
https://manual.limesurvey.org/BeforeToolsMenuRender
must be removed and use exactly a new 'global event' to add items to an existing menu (and subitem too)

I'm unsure for this
https://manual.limesurvey.org/AfterQuickMenuLoad
https://manual.limesurvey.org/BeforeSideMenuRender

We are always happy to have pull request and discussion to improve LS core :)

YuriyBabenko

YuriyBabenko

2016-09-23 09:36

reporter   ~40928

@DenisChenu - a single event for all menus is fine too, but only if the same data structure is expected for all menu types.

Currently, beforeAdminMenuRender() must set 'extraMenus' to an array of \ls\menu\Menu items, but beforeToolsMenuRender() must set 'menuItems' to an array of \ls\menu\MenuItem items. This disconnect would cause problems if we were to use a single event handler for all admin menus. I think some refactoring and standardization is needed here.

DenisChenu

DenisChenu

2016-09-23 09:44

developer   ~40929

" I think some refactoring and standardization is needed here. "
+1

ollehar

ollehar

2016-09-23 10:20

administrator   ~40930

Hi Yuriy,

First of all: What's your use-case?

Second: Yes, beforeAdminMenuRender() and beforeToolsMenuRender() accept different objects, but that is because the first renders a new menu entry, the second just adds menu items to an existent menu entry. That's why beforeToolsMenuRender() accepts menuItem objects.

DenisChenu

DenisChenu

2016-09-23 11:53

developer   ~40933

Last edited: 2016-09-23 11:53

View 2 revisions

@olle : question :

  1. Are we able today to add a menu in all menu wrapper ?
  2. Are we able today to add a menu-item in all menu ?
    (or near all menu, maybe some exception due to some refactorisation to do).

If no : do you think it can be done to update this 2 event to add "what menu wrapper (some menu-wrapper id) + object name + object id

for example :'menu'=>'participantbar','object'=>'participant','objectid'=>(isset($participantid)?$participantid:null,

Somethink like this ?

After we can remove all extra menu/bar events

YuriyBabenko

YuriyBabenko

2016-09-23 19:10

reporter   ~40934

@olle - My use case is adding a new link to the survey bar. This link will show a modal that will allow re-using existing questions/answers from previous surveys in the current survey. All that will be handled in the plugin and I've PoC'd it, but adding the link is where I got stuck yesterday.

YuriyBabenko

YuriyBabenko

2016-09-24 02:20

reporter   ~40935

I just created a PR with a basic implementation of the missing event: https://github.com/LimeSurvey/LimeSurvey/pull/537

ollehar

ollehar

2016-09-26 10:27

administrator   ~40952

That's nice, but as I said in the comment section on GitHub, I already implemented this feature in the cint branch some weeks ago. Could you please have a look at that and see if it fits all your requirements?

YuriyBabenko

YuriyBabenko

2016-09-27 05:25

reporter   ~40963

@olle - no, the code on the 'cint' branch does not work for our needs.

Two problems:

  1. The only data Survey_Common_Action.php shares with the event is the survey ID. By not having access to the main $aData array (and subsequently, `$aData['surveybar']), we cannot conditionally add new buttons based on the presence (or lack) of existing buttons. So we cannot write a plugin that will add a "reuse existing question" button whenever a "add new question" button exists.

We can sorta-kinda get around this by checking the URI of the current page and making assumptions, but that's an ugly solution.

  1. The rendering of $beforeSurveyBarRender is nested under the condition on line 72 (if(isset($surveybar['buttons']['view']))), which severely restricts when extra surveybar buttons can be output. For example, this code never gets called on the "List Questions" page of a survey.
ollehar

ollehar

2016-09-27 10:11

administrator   ~40969

Thanks for your feedback.

About 1, how about adding two additional inputs to the event: controller name and action name? You can then use that to decide when to add additional buttons.

  1. Will have a look at this.
DenisChenu

DenisChenu

2016-09-27 10:15

developer   ~40970

Last edited: 2016-09-27 10:15

View 2 revisions

For "controller name and action name" : yes , must be. object and object id too ? ;)

ollehar

ollehar

2016-09-27 10:22

administrator   ~40971

Denis, id of which object?

Yuriy, about 2, what about moving the rendering to just before "right action buttons"?

YuriyBabenko

YuriyBabenko

2016-09-27 22:07

reporter   ~40988

@olle - Controller and Action names would be nice. I'm guessing @DenisChenu was referring to the model object (Survey, in this case) and its ID.

Also, there's a bug with the logic in surveybar_view.php:

There's a check for if ($menu->isDropDown()), and in the else condition, a call to $menu->getIconClass(). The Menu class does not have a getIconClass() method.

YuriyBabenko

YuriyBabenko

2016-09-27 22:11

reporter   ~40989

Also - moving the view logic to just before "right action buttons" gets around the problem of being nested under a false condition, but this results in the buttons being printed out in a new DIV, bumping them down to a new line. I can get around this with CSS (floating the two containers), but that's not ideal.

ollehar

ollehar

2016-10-20 15:16

administrator   ~41507

@Yuriy, are you working on this still?

Issue History

Date Modified Username Field Change
2016-09-23 04:38 YuriyBabenko New Issue
2016-09-23 08:51 DenisChenu Note Added: 40925
2016-09-23 08:51 DenisChenu Assigned To => ollehar
2016-09-23 08:51 DenisChenu Status new => assigned
2016-09-23 09:08 YuriyBabenko File Added: survey bar menu.png
2016-09-23 09:11 YuriyBabenko Note Added: 40926
2016-09-23 09:32 DenisChenu Note Added: 40927
2016-09-23 09:32 DenisChenu Note Edited: 40927 View Revisions
2016-09-23 09:36 YuriyBabenko Note Added: 40928
2016-09-23 09:44 DenisChenu Note Added: 40929
2016-09-23 10:20 ollehar Note Added: 40930
2016-09-23 11:53 DenisChenu Note Added: 40933
2016-09-23 11:53 DenisChenu Note Edited: 40933 View Revisions
2016-09-23 19:10 YuriyBabenko Note Added: 40934
2016-09-24 02:20 YuriyBabenko Note Added: 40935
2016-09-26 10:27 ollehar Note Added: 40952
2016-09-27 05:25 YuriyBabenko Note Added: 40963
2016-09-27 10:11 ollehar Note Added: 40969
2016-09-27 10:15 DenisChenu Note Added: 40970
2016-09-27 10:15 DenisChenu Note Edited: 40970 View Revisions
2016-09-27 10:22 ollehar Note Added: 40971
2016-09-27 22:07 YuriyBabenko Note Added: 40988
2016-09-27 22:11 YuriyBabenko Note Added: 40989
2016-10-20 15:16 ollehar Note Added: 41507
2016-10-20 15:16 ollehar Status assigned => feedback