View Issue Details

IDProjectCategoryView StatusLast Update
16702Development Survey editingpublic2020-12-03 16:28
Reporterollehar Assigned Toollehar  
PrioritynoneSeverityfeature 
Status assignedResolutionopen 
Summary16702: Replace VueJS top-bar with Yii widget
Description

Alternatives:

  1. Keep current Vue component (but possible improve it/make it easier to add/remove buttons)
  2. Use raw HTML (forwardporting LS3, compare: https://github.com/LimeSurvey/LimeSurvey/blob/3.x-LTS/application/views/admin/survey/Question/questionbar_view.php) (pro: straigh-forward, con: too much logic in viewfiles?)
  3. Use CMenu Yii widget
  4. Extend CMenu Yii widget and use objects instead of associative arrays to define the menu data
  5. As 4, but extend CWidget (CMenu does not contain much that we need, and doesn't use view files)

Some brainstorming being done.

https://www.yiiframework.com/doc/api/1.1/CMenu

Personally I am not a fan of using associative arrays as data carriers. I don't believe they scale well in huge projects.

Alternative to 4 is casting plugin Menu objects to arrays that CMenu can understand. Like $menuArray = (array) $menu. Or using built-in ArrayAccess interface.

The top-bar should do only one thing - display buttons that are fed to it. the different types of buttons (or widgets, since condition designer also has a question selector) correspond to different classes carrying the data. the mapping from object data to HTML is done in the widget view files.

E.g., if you have a DropDownMenu class, then you'll have a dropdownmenu.php view file in the widget.

So in the controller view you end up with

?php $this->widget('ls.widget.topbar', [$dropdownmenu1, $dropdownmenu2, $separator, $menuitem]); ?

Branch: feature/16702-replace-vuejs-topbar-with-widget

TagsNo tags attached.

Relationships

related to 16784 assignedgabrieljenik Bug reports Top-bar missing buttons when adding new question group 

Activities

ollehar

ollehar

2020-09-29 11:53

administrator   ~59977

@DenisChenu Time to review?

ollehar

ollehar

2020-09-29 12:54

administrator   ~59978

Last edited: 2020-09-29 13:54

View 4 revisions

Not sure how to make CMenu play well with Bootstrap.

Edit: Answer: htmlOptions => [class => btn]

Edit 2: But HTML structure is different than CMenu.

Edit 3: Can use item templates for this. ^^

ollehar

ollehar

2020-09-29 15:11

administrator   ~59979

Example (unfinished) implementation using CMenu:

$this->widget('zii.widgets.CMenu', [
    'htmlOptions' => [
        'class' => 'nav navbar-nav scoped-topbar-nav ls-flex-item ls-flex-row grow-2 text-left'
    ],
    'items' => [
        [
            'label'       => 'Activate survey',
            'url'         => ['site/index'],
            'template'    => '<div class="topbarbutton">{menu}</div>',
            'linkOptions' => ['class' => 'btn btn-primary topbarbutton' ]
        ],
        [
            'label' => 'Preview survey',
            'url'   => ['product/index'],
            'template' => '&lt;div class=&quot;topbarbutton&quot;>&lt;div class=&quot;btn btn-default&quot;>&lt;i class=&quot;fa fa-cog&quot;></i>&nbsp;{menu}&lt;/div>&lt;/div>',
            'items' => [
                [
                    'label' => 'New Arrivals',
                    'url'   => ['product/new', 'tag'=>'new']
                ],
                [
                    'label' => 'Most Popular',
                    'url'   => ['product/index', 'tag' => 'popular']
                ],
            ]
        ],
        [
            'label' => 'Login',
            'url'   => ['site/login']
        ]
    ]
]);

Problems: How to implement Bootstrap HTML structure? How to replace anchor with button tag for dropdown menu?

gabrieljenik

gabrieljenik

2020-09-29 15:20

developer   ~59980

I like CMenu. I haven't used it in a while.
I would go for option 3.

Still some comments.
Use raw HTML (forwardporting LS3. Keep in mind that file is not just for rendering. Permissions are checked.
So, we will need an extra component that will set the menu-item-array according to the action and the user profile.
Maybe could be a base controller method linked to a helper class? Maybe it could be an extension from CMenu that receives the action and the user as parameters? (hooking from beginContent)

Extend CMenu Yii widget and use objects instead of associative arrays to define the menu data I believe we could handle that on later stages. What would be the differences between objects and arrays? arent' both sent as reference (if remember correctly)?

As 4, but extend CWidget (CMenu does not contain much that we need, and doesn't use view files) We can use itemTemplate (https://www.yiiframework.com/doc/api/1.1/CMenu#itemTemplate-detail) and getViewFile (https://www.yiiframework.com/doc/api/1.1/CWidget#getViewFile-detail). Honestly, we would need to run a PoC first, but I am very confident this works well with bootstrap. I even remember the default app from Yii uses bootstrap.

As for the question selector, we could use a custom rendered and in there make a query for fetching the questions. Then we would minimize the data passed as parameters.

I would dig a little bit more on the cMenu and look for examples with permission checking.

ollehar

ollehar

2020-09-29 15:24

administrator   ~59981

Last edited: 2020-09-29 15:30

View 2 revisions

CMenu test here: https://github.com/LimeSurvey/LimeSurvey/commit/1f29debbd7fb03635b1f1140789dc8691d3b9533

Someone will have to try to get this to work. Otherwise, I vote against.

ollehar

ollehar

2020-09-29 15:30

administrator   ~59982

Last edited: 2020-09-29 16:06

View 3 revisions

What would be the differences between objects and arrays?

I should write an article about this, but with objects you can do:

  • Validation
  • Default values
  • Inheritance, interfaces, polymorphism
  • Immutability
  • Encapsulation
  • Split up in files
  • Self-documentation and documentation
  • Type-safety ("tag" the data you're passing around, expressing intent more clearly)

There's no performance difference between arrays and objects in PHP.

Arrays have value semantics, objects are passed as references.

ollehar

ollehar

2020-09-29 16:01

administrator   ~59983

Last edited: 2020-09-29 16:06

View 3 revisions

"PHP: Use associative arrays basically never": https://steemit.com/php/@crell/php-use-associative-arrays-basically-never

In just about every other situation I can think of, named classes win. Their memory usage is half that of a corresponding array. The optimizations the engine can do when it knows up front what the structure of your data is going to be are massive and pay off huge dividends in memory consumption. They're also over 10% faster. The only downside is when trying to serialize them when there is an added cost to time, memory, and stored size. When we also consider that a classed object is far more self-documenting than an associative array, gives IDEs the ability to auto-complete for you, and gives you a place to include additional documentation (which you should include), it's one of the clearest wins I've seen in PHP.

But also:

The first thing we can conclude is that if the one and only thing you care about is serialization/deserialization performance, associative arrays still win. They're the most time efficient by more than 50%, and the most space efficient by up to 20%.

DenisChenu

DenisChenu

2020-09-30 15:15

developer   ~59994

I don't really care on the final solution ;)

But something that can be extendable … or user manageable …

Create our own widget (or extende CMenu) based on https://github.com/LimeSurvey/LimeSurvey/tree/master/application/libraries/MenuObjects ?

ollehar

ollehar

2020-09-30 16:10

administrator   ~59995

Yes, my widget uses the same classes as the ones for the plugins - Menu and MenuItem (and others inherited from those).

ollehar

ollehar

2020-10-01 10:19

administrator   ~59999

@gabrieljenik Can you push the code you made, too, using CMenu? So it can be reviewed. :)

gabrieljenik

gabrieljenik

2020-10-01 14:29

developer   ~60016

Last edited: 2020-10-01 14:30

View 2 revisions

$this->widget('zii.widgets.CMenu', [
            'htmlOptions' => [
                'class' => 'nav navbar-nav scoped-topbar-nav ls-flex-item ls-flex-row grow-2 text-left'
            ],
            'items' => [
                [
                    'label'       => 'Activate survey',
                    'url'         => ['site/index'],
                    'template'    => '&lt;div class=&quot;topbarbutton&quot;>{menu}&lt;/div>',
                    'linkOptions' => ['class' => 'btn btn-primary topbarbutton' ]
                ],
                [
                    'label' => 'Preview survey',
                    //'url'   => ['product/index'],
                    'template' => '&lt;button class=&quot;btn btn-default dropdown-toggle&quot; type=&quot;button&quot; data-toggle=&quot;dropdown&quot;>&lt;i class=&quot;fa fa-cog&quot;></i> {menu} &lt;i class=&quot;caret icon&quot;></i>&lt;/button>',
                    'submenuOptions' => [
                        'class' => 'dropdown-menu'
                    ],
                    'itemOptions' => [
                        'class' => 'topbarbutton dropdown'
                    ],
                    'items' => [
                        [
                            'label' => 'New Arrivals',
                            'url'   => ['product/new', 'tag'=>'new']
                        ],
                        [
                            'label' => 'Most Popular',
                            'url'   => ['product/index', 'tag' => 'popular']
                        ],
                    ]
                ]
            ]
        ]);
image.png (12,041 bytes)   
image.png (12,041 bytes)   
ollehar

ollehar

2020-10-01 14:38

administrator   ~60020

Nicely done, Gabriel! However, one problem I see with this approach is that HTML ends up in strings among programming logic. Is it possible to move template to a view file, for example? HTML in strings have drawbacks compared to HTML in view files, e.g. lacking syntax highlight, indentation. In the long run, I believe this hurts maintainability and readability.

Another alternative would be to have a new class "CMenuItem` that implements ArrayAccess interface. This way, we could get rid of long, associative arrays.

DenisChenu

DenisChenu

2020-10-01 14:42

developer   ~60024

Is it possible to move template to a view file, for example?

:+1: It can be really great :)

gabrieljenik

gabrieljenik

2020-10-01 15:49

developer   ~60030

@ollehar, Yes this need improvement. We were just doing it as a Proof of Concept for the dropdowns.
So, netx step is separating html and logic as much as possible.

ollehar

ollehar

2020-10-01 15:50

administrator   ~60031

@gabrieljenik The thing is, as you work through this, I think in the end you will re-implement the solution I already did with CWidget. :) Which has classes and view files.

gabrieljenik

gabrieljenik

2020-10-01 16:12

developer   ~60032

Last edited: 2020-10-01 16:13

View 2 revisions

Well... that could be true... :) Not sure how much advanced is that solution, but both go in the same direction.
So you let me know which direction we take...

ollehar

ollehar

2020-10-01 16:14

administrator   ~60033

We'll discuss in the team and let you know. Thanks!

ollehar

ollehar

2020-10-02 12:27

administrator   ~60047

Seems like we can't agree right now, so we'll go back to LS3 style, with buttons in view files. No widget.

gabrieljenik

gabrieljenik

2020-10-02 14:54

developer   ~60054

Good. Will adapt question groups toolbars and others from the screens I ported

ollehar

ollehar

2020-10-02 14:55

administrator   ~60055

Related discussion of my attempt: https://forum.yiiframework.com/t/renderobjectasviewwidget/130644

ollehar

ollehar

2020-10-20 13:27

administrator   ~60302

Notes from Patricia:

We decided to use the topbar from LS3 (in conception meeting) in first step.
These are the relevant "new" things in this regard:

in an action there must be the line:
$aData['renderSpecificTopbar'] = 'copyQuestiontopbar_view';
The value 'copyQuestiontopvar_view' is the name of the view where the specific topbar is saved. This topbar is rendered directly in the view
$this->render('copyQuestionForm', $aData);
by using the function renderPartial('topbars/'.'nameOfTopbarView', $data)The view for the topbar is in the folder views/controllerName/topbars (e.g. /views/questionAdministration/topbars/copyQuestiontopbar_view). So every controller can have more then one topbar (depending on the action). Of course some actions can also use the same topbar if needed. That prevents using always "if...else".

Advantages:
-- no special call to topbar function anymore in layout_questioneditor (except those we still have now)
-- topbars could always be searched&found at the same place (views/controllerName/topbars/sometopbar.php)
-- buttons can be added easily
-- permissions can be checked in the controllers action directly (no permission to use the action --> no view is rendered --> no buttons are shown)

gabrieljenik

gabrieljenik

2020-10-21 00:46

developer   ~60321

Any branch where we can see an example?

ollehar

ollehar

2020-10-21 14:08

administrator   ~60332

Ping @pstelling

pstelling

pstelling

2020-10-22 11:21

developer   ~60352

it's in the branch "https://github.com/LimeSurvey/LimeSurvey/tree/task/zoho-L41-T55-topbar-copy-question&quot;

ping @gabrieljenik

gabrieljenik

gabrieljenik

2020-10-28 14:22

developer   ~60444

Hi @pstelling

Thanks for the code.
Please, look at the attachment. This is how it looks when the page is loaded through the sidebar w/pjax.
I think we need to work on layout for some pages (I think it is the same layout for survey details) and remove the topbar from it so the top bar is loaded by pjax only (second toolbar), right?
Also, some styling needs to be twitched, right?

On the second screenshot we can see everything makes sense as the layout is different.

Thoughts?
Should we go ahead with it?

image-2.png (20,727 bytes)   
image-2.png (20,727 bytes)   
image-3.png (33,198 bytes)   
image-3.png (33,198 bytes)   
pstelling

pstelling

2020-10-28 20:37

developer   ~60446

Hi Gabriel,

thanks for the hint. Yes obviously there is a problem when rendering something with Ajax Request. Did you change/implement the topbar in "Edit email templaztes"?
I'll check this. Maybe we will discuss again about topbar solutions.

Bye
Patricia

gabrieljenik

gabrieljenik

2020-10-29 16:42

developer   ~60449

Yes, for email templates we created a top bar, copying it from LS3.
I agree, not the best one :)

We could actually add some stuff on it. But that's enhancements.

Look forward to your comments on how to move forward.
I guess changing some layouts is involved.
But will wait for your comments.

Thanks

pstelling

pstelling

2020-10-30 08:40

developer   ~60450

Hi Gabriel,

thanks for your comment :). We had the idea to stay with the "design" (layout) for the topbar like it is in LS3. Would you like to have another design/layout for it? What would you prefer?

What kind of "stuff" would you like to add? Enhancements are always good. Best when we get information about it. Would you like to write it (e.g. Buttons missing or would be nice to have the Button X ). Would also be nice to know the "intention" behind, like "having this button in addition gives us the possibility of ...".

Of course there should not be two topbars (one behind the other, like in image2). Can you send me the link to the branch where you implemented the topbar for email templates?
Thanks a lot for all the information you give us in this regard.

I'm looking forward to hear from you.

Best regards
Patricia

gabrieljenik

gabrieljenik

2020-10-30 13:23

developer   ~60462

What kind of "stuff" would you like to add? Enhancements are always good. Best when we get information about it. Would you like to write it (e.g. Buttons missing or would be nice to have the Button X ). Would also be nice to know the "intention" behind, like "having this button in addition gives us the possibility of ...".

I was referring to the email template toolbar, not the general toolbar system

Of course there should not be two topbars (one behind the other, like in image2). Can you send me the link to the branch where you implemented the topbar for email templates?

Sorry, I missed the link on this ticket.
https://github.com/LimeSurvey/LimeSurvey/pull/1634

Let me know if you want to hook into a call.

Thanks!

gabrieljenik

gabrieljenik

2020-11-06 19:58

developer   ~60584

Added some code here:
https://github.com/LimeSurvey/LimeSurvey/pull/1648

Don't kill me :)
I have taken the concept a little bit further as:

  • some topbars need to be shared among views (they are the same)
  • need some common code in between all topbars as to reuse code and reduce maintenance efforts.

Still, everything is overridable, so if we need a specific topbar for a single view, we can do that.
I integrated it with the LSBaseController and the new rendering mechanisms.

Double topBars is solved.

This a WIP, I am working on the surveyTopBar which is very general.

DenisChenu

DenisChenu

2020-11-08 15:16

developer   ~60586

some topbars need to be shared among views (they are the same)

+1, but i don't like &lt;?= $rightSideContent ?>

I prefer params, @patricia start something here : https://github.com/LimeSurvey/LimeSurvey/blob/develop/application/views/admin/super/fullpagebar_view.php

I use it in views (and adapt send button)
https://github.com/LimeSurvey/LimeSurvey/pull/1632/files#diff-826c61128100ca4f347af88c674f69e38d2427d3fe6323cfe39807444e01d958R20

gabrieljenik

gabrieljenik

2020-11-09 13:22

developer   ~60592

I prefer params, @patricia start something here : https://github.com/LimeSurvey/LimeSurvey/blob/develop/application/views/admin/super/fullpagebar_view.php

Those kind of params are something to come. Definitely.
That $rightSideContent is rendered by a view whose name is sent by a parameter.

That rightSideContent redended can be on top of a blankTopBar or on top of a shared top bar with some default buttons already.

DenisChenu

DenisChenu

2020-11-09 13:25

developer   ~60593

I think it's totally related to the Menu project from @ollehar too.

Finally : all top tools are menu …

gabrieljenik

gabrieljenik

2020-11-09 13:39

developer   ~60594

I think it is the same!
Actually, I understand that project concluded into this.

gabrieljenik

gabrieljenik

2020-12-03 16:28

developer   ~60823

Turned configuration into a class and rendering enclosed into a widget:
https://github.com/LimeSurvey/LimeSurvey/pull/1648

Issue History

Date Modified Username Field Change
2020-09-28 12:41 ollehar New Issue
2020-09-28 12:41 ollehar Description Updated View Revisions
2020-09-28 12:42 ollehar Description Updated View Revisions
2020-09-28 12:42 ollehar Description Updated View Revisions
2020-09-28 12:44 ollehar Description Updated View Revisions
2020-09-28 12:44 ollehar Description Updated View Revisions
2020-09-28 15:31 ollehar Description Updated View Revisions
2020-09-29 11:49 ollehar Description Updated View Revisions
2020-09-29 11:53 ollehar Note Added: 59977
2020-09-29 12:09 ollehar Description Updated View Revisions
2020-09-29 12:24 ollehar Assigned To => ollehar
2020-09-29 12:24 ollehar Status new => assigned
2020-09-29 12:54 ollehar Note Added: 59978
2020-09-29 13:11 ollehar Note Edited: 59978 View Revisions
2020-09-29 13:21 ollehar Note Edited: 59978 View Revisions
2020-09-29 13:54 ollehar Note Edited: 59978 View Revisions
2020-09-29 14:55 ollehar Description Updated View Revisions
2020-09-29 15:11 ollehar Note Added: 59979
2020-09-29 15:20 gabrieljenik Note Added: 59980
2020-09-29 15:24 ollehar Note Added: 59981
2020-09-29 15:30 ollehar Note Added: 59982
2020-09-29 15:30 ollehar Note Edited: 59981 View Revisions
2020-09-29 15:41 ollehar Note Edited: 59982 View Revisions
2020-09-29 16:01 ollehar Note Added: 59983
2020-09-29 16:05 ollehar Note Edited: 59983 View Revisions
2020-09-29 16:06 ollehar Note Edited: 59983 View Revisions
2020-09-29 16:06 ollehar Note Edited: 59982 View Revisions
2020-09-30 15:15 DenisChenu Note Added: 59994
2020-09-30 15:15 DenisChenu File Added: Capture d’écran du 2020-09-30 15-12-51.png
2020-09-30 16:10 ollehar Note Added: 59995
2020-10-01 10:19 ollehar Note Added: 59999
2020-10-01 14:29 gabrieljenik Note Added: 60016
2020-10-01 14:29 gabrieljenik File Added: image.png
2020-10-01 14:30 gabrieljenik Note Edited: 60016 View Revisions
2020-10-01 14:38 ollehar Note Added: 60020
2020-10-01 14:42 DenisChenu Note Added: 60024
2020-10-01 15:49 gabrieljenik Note Added: 60030
2020-10-01 15:50 ollehar Note Added: 60031
2020-10-01 16:12 gabrieljenik Note Added: 60032
2020-10-01 16:13 gabrieljenik Note Edited: 60032 View Revisions
2020-10-01 16:14 ollehar Note Added: 60033
2020-10-02 12:27 ollehar Note Added: 60047
2020-10-02 14:54 gabrieljenik Note Added: 60054
2020-10-02 14:55 ollehar Note Added: 60055
2020-10-14 11:18 ollehar Description Updated View Revisions
2020-10-20 13:27 ollehar Note Added: 60302
2020-10-20 13:28 ollehar Project Feature requests => Development
2020-10-21 00:46 gabrieljenik Note Added: 60321
2020-10-21 14:08 ollehar Note Added: 60332
2020-10-22 11:21 pstelling Note Added: 60352
2020-10-28 14:06 gabrieljenik Relationship added related to 16784
2020-10-28 14:22 gabrieljenik Note Added: 60444
2020-10-28 14:22 gabrieljenik File Added: image-2.png
2020-10-28 14:22 gabrieljenik File Added: image-3.png
2020-10-28 20:37 pstelling Note Added: 60446
2020-10-29 16:42 gabrieljenik Note Added: 60449
2020-10-30 08:40 pstelling Note Added: 60450
2020-10-30 13:23 gabrieljenik Note Added: 60462
2020-11-06 19:58 gabrieljenik Note Added: 60584
2020-11-08 15:16 DenisChenu Note Added: 60586
2020-11-09 13:22 gabrieljenik Note Added: 60592
2020-11-09 13:25 DenisChenu Note Added: 60593
2020-11-09 13:39 gabrieljenik Note Added: 60594
2020-12-03 16:28 gabrieljenik Note Added: 60823