Option to render the subitems as a Dropdown (or not) #35
Conversation
| } | ||
|
|
||
| /** | ||
| * Renders widget items. | ||
| */ | ||
| public function renderItems() | ||
| public function renderItems($items, $options = []) |
There was a problem hiding this comment.
It is not good to change public method signature.
It is better to extract some method like renderItemsInternal(), which can be used at renderItem() and at renderItems()
|
@klimov-paul You could have made the comments before, when you told me to redo the PR and I had the job of making the merge 😅 |
|
Sorry |
|
@klimov-paul Your comment was very good! Check it out and tell me what you think. |
| @@ -164,6 +174,7 @@ public function renderItem($item) | |||
| $items = ArrayHelper::getValue($item, 'items'); | |||
| $url = ArrayHelper::getValue($item, 'url', '#'); | |||
| $linkOptions = ArrayHelper::getValue($item, 'linkOptions', []); | |||
| $asDropdown = isset($item['inline']) ? !$item['inline'] : $this->asDropdown; | |||
There was a problem hiding this comment.
Diffrent names 'inline' and 'asDropdown' will confuse developer. Single one should be used both for default optnion property and item option key.
There was a problem hiding this comment.
I let different because they are different things. One is a global property of the widget, another is an option item.
You think it might be the same name?
There was a problem hiding this comment.
It must be same or similar. 'dropdown' can be used for item, for example.
There was a problem hiding this comment.
See if it was good now, please.
|
@klimov-paul See if it was good now, please. |
| @@ -66,6 +67,10 @@ class Nav extends Widget | |||
| */ | |||
| public $items = []; | |||
| /** | |||
| * @var boolean whether the nav subitems should be a [[Dropdown]] widget. | |||
| */ | |||
| public $asDropdown = true; | |||
There was a problem hiding this comment.
I am unsure about name for this option.
Curently it does not sound very good. Must be something consistent with encodeLabels, but I can't find a good name.
There was a problem hiding this comment.
Hard decision! 😆
How about $itemsAsDropdown?
|
Rendering Now, I am not sure if such option should be supported at core level. |
|
@klimov-paul this option is set according to the application menu. On a sidebar, the sub-items must be in the list. |
|
Can you provide an example link? |
|
@klimov-paul http://www.keenthemes.com/preview/metronic/theme/templates/admin4/ @samdark renamed to useDropdown |
|
May be I am missing something, but this link does not look convincing for me. I am lack of exprtise here. Someone else should look into this. |
|
I see no problem here and I do not understand your concern. By default, the result is the same. If necessary, a parameter is included and we have the possibility of using the navigation sidebar. Without this PR, the sidebar is frustrated. Perhaps @cebe can give us help. |
|
The problem is that without extra adjustments 'inline' rendering produces HTML code, which have a poor appearance. |
But that is precisely the goal of PR: To give the option to those in need. Who does not need, does not touch anything. All developers who use a sidebar for navigation need. |
(@klimov-paul yiisoft/yii2#4346)
Navigation is not always done with Dropdown.
On the website "Themeforest" many themes for the Bootstrap, use a sidebar for navigation without Dropdown.
It's nice to have the option to also generate sub items as a list.
Even if the implementation is not the way I propose, would be very important to have this option.