@matrikular Can you please describe what are the Chances to expect if assigning images, changing the target window / method and so on?

@franz-wohlkoenig First of all; thank you for your time and interest.

Doing all that stuff you would (can) normally do with those menu types, should just work as before. Nothing to expect but the same result. Say, if you assigned an image to the menu item and it showed up before the patch, it should also show after the patch. The same goes for changing the target window parameter and so on.

What has changed is really just the implementation of the same bevahior. One exmple:
default_url.php (old) vs. default_url.php (new)
Hint: Links open in the same window.

I have tested this item successfully on ef3bf60

Tested "System Links":
1. Text Separator: with/without Link Image / Add Menu Title / Link CSS Style / Display in Menu
2. Menu Item Alias: same as 1. + Target Window / Link Title Attribute
3. Menu Heading: same as 1.
4. External URL: same as 2. + Link Rel Attribute

Tested Component "Weblinks"
1. Submit a Weblink: with/without Target Window / Link Title Attribute / Link CSS Style / Link Image / Add Menu Title / Display in Menu
2. List Web Links in a Category: same as 1.
3. List All Web Link Categories: same as 1.

Module mod_menu set "Show Sub-menu Items" to "Yes":

Without Patch

With Patch (yeah)

(can't upload Images on Issue Tracker).

I have tested this item successfully on ef3bf60

successfully tested

Not sure if this needs to be reviewed for b/c as there are some markup changes

Problem: If you use this a small width devices, then the hover menu appears outside of the view.

This PR has received new commits.

CC: @franz-wohlkoenig, @mikeveeckmans

This PR has received new commits.

CC: @franz-wohlkoenig, @mikeveeckmans

I did a review of the files and it looks fine. Markup shouldn't change (anymore) with the latest commits.

The code indeed is easier to read now and makes use of JHtml methods. So from me.

Thank you very much for your input both @ufuk-avcu and @Bakual

I would have argued that the way the templates render or style different kinds of content is a thing to discuss in itself and has little to do with the patch.

Though, for the sake of testing ("b/c"), I've reverted the changes for the default.php layout so that the dropdown behavior should be gone or, back to its old state (== removed the drowpdown CSS classes from each entry). Since protostar tries to use so much of the bootstrap behavior and also applies the expected styles like " nav-pill", I find the implementation of many things not quite right.

That said, the default.php layout, which now only differs in the the way it is written, not the output, wasn't really in the main scope of this PR.

I hope that the code changes show why:
<ul class="nav menu<?php echo $class_sfx;?>"<?php
$tag = '';
if ($params->get('tag_id') != null)
{
$tag = $params->get('tag_id') . '';
echo ' id="' . $tag . '"';
}
?>>
is harder to read and / or maintain than:
<ul class="nav menu<?php echo $class_sfx; ?>"<?php echo $id; ?>>

or why this version of default_url.php is harder to read and / or maintain than the proposed one.

It might sound silly / stupid to use buzzwords like dry, kiss and all the other acronyms in the context of such trivial thing like a layout but, ... for the sake of Joomla! 4++ - we should consider not to write code like this or this anymore.

Thank you for your time.

I have tested this item successfully on 56f8d1b

I have tested this item successfully on 56f8d1b

TEST OK

RTC - thanks