@brianteeman is this using only css or also some BS component (tooltip, popover)?

@dgrammatiko only css - the tooltips are the same as in the admin

Noice ?

Related #30639

This is now ready for review
Any and all code optimisations welcome

I have tested this item ✅ successfully on bf2a2e8

can someone please add the release blocker tag

As all the css is in a state of uncertainty regarding bs5 that can be addressed in another PR.

Shall we leave the issue #30639 until that has been done?

I have tested this item ✅ successfully on 4ec5fec

I can confirm that the editing links and and tooltips are right and work as expected, in case od module edit for both settings "Site" and "Admin" in com_module's options, so that all remaining issues are CSS.

Especially for the dropdown menu (metismenu) of the Cassiopeia template it needs fixes, beside what is mentioned in the description of this PR.

Last change after @dgrammatiko 's test was only code style for a code comment, so the test is still valid. I've restored the test result in the issue tracker.

RTC

Last change again was only code style for a code comment, so previous tests and RTC status are still valid.

Just to mention, are this (I think valid) points are taken in considerations? #30639 (comment)

@bembelimen I don't see any way that the imho stupid menu editing will ever be great but at least this is accessible

The css does need a lot of work but its beyond my skillset and not something that can really be addressed until the bs5 decision is made. Both the menu and module links are not reliably rendered on many templates right now anyway as the template designers rarely take this into consideration

I am not convinced with this solution. I had it too but i gave up. My test looks like this: And some items are not accessible for the mouse ...

Maybe @drmenzelit can help with the CSS?

Maybe we continue the work here but as a DRAFT @brianteeman? The first step with killing the js is done.

I have not tested this item.

It seems I haven't tested well enough with my previous test. I haven't checked accessibility with keyboard. Sorry.

@bembelimen I don't see any way that the imho stupid menu editing will ever be great but at least this is accessible

Thanks for saying that, yes that is the biggest problem.

Perhaps there is some designer out there, who could help improving the visible stuff (code looks good).

Back to pending.

@chmst As i stated several times the css is not correct.

It is easier if the menu link doesn't have any popover at all as I don't see the need for it but my aim was to make the current information accessible - not to remove features - but I am happy to do so.

@richard67 whats wrong with the keyboard?

Even without the menu popover there is still a css issue with the module link placement. (Not related to this PR as the problem existed without this PR)

@brianteeman ist is not about the css, iI don't care about. it is about the popovers. In the right dropdown menu the popvers block the mouse

@richard67 whats wrong with the keyboard?

Silly me. Was reading the comment above too fast. Was about mouse, not keyboard. I just thought I haven't tested well enough.

@chmst they are positioned by css ;)

I have pushed an update which moves the popover into an aria-label. This resolves the mouse issue that @chmst refers to and yet ensure that the links are accessible with unique names

Found another bug. The code to detect a module was not correct and it didnt find the search module. I've fixed that now as well - I think there was a previous report about that

The code to detect a module was not correct and it didnt find the search module. I've fixed that now as well - I think there was a previous report about that

FWIW the code is unnecessary opinionated with the regex approach. A better approach would be to use DOMDocument to parse and find the needed elements (eg an element could be a custom element with any kind of name like some-name). But let's leave this for another PR. Just wanted to share this here

Maybe I miss something, but could you please make your right navigation module a dropdown module, please? Then the dropdown and the edit button are in clinch and it is not possible to klick the module edit button.

@dgrammatiko That's the code that was there before - I just made it work correctly

@chmst like this ??

That's the code that was there before - I just made it work correctly

@brianteeman I'm not blaming or even asking for changes here. This was a footnote and mostly a reminder to myself to revisit this code once this PR get's merged

right rop - edit the module,
the edit menuItems are fine without tooltips

For me the outstanding issues are

1. The css that is placing the links on the page is wrong OR its highlighting an html error as the links for the breadcrumbs are not inside the breadcrumbs but located top right of the page
2. The css that is placing the tooltip is wrong as its disconnected from the link

Maybe it can resolved with css.

From my point of view - but that is only my personal taste maybe - the menu item edit icons would look better if there were not appended but prepended to the menu item title, so that (when LTR) both the edit icons and the titles of menu items of the same level would all be left aligned below each other.

This could also make CSS for the menu item edit icons much easier for the dropdown menus, where we have 2 different kinds of parent items, such where the menu item title is a link and the dropdown toggle is a small icon, and such where the whole menu item is the dropdown toggle because there is no link (i.e. headings and separators).

@brianteeman What do you think?

@brianteeman What do you think?

If it was just the icon then yes - but as its an icon + text then no

At the end of the day I dont see any way this will ever look nice (like language debug) but this should at least make it functional

As it's mainly a feature used by site admins and not by normal content editors so it can be switched off, I think it doesn't need to look nice, it only has to be functional.

If it was just the icon then yes - but as its an icon + text then no

And if we put some border around the icon and the "Edit" text so both together look like a button?

Like I did for the module edit?

Like I did for the module edit?

I haven't tested yet, but in code it looks like what I have in mind.

Like I did for the module edit?

I haven't tested yet, but in code it looks like what I have in mind.

Update: I've just tested. Yes, something like that, just a bit smaller.

@brianteeman Other idea for menu item edit: Just show the icon but not the "Edit" text, i.e. make the "Edit" sr-only, and render icon and (sr-only) text before the menu item title? That would look nice, too.

Update: Or just leave away that "Edit" text at all as we already have an aria-label telling screen readers what it is.

@brianteeman I would not call it "requested", not even "recommended", but "suggested" ;-)

Anyway, I like it:

What do others think?

Rest is CSS fine tuning ... maybe a bit padding between the edit icon and the menu item title for thick fingers on touch screens / mobile clients.

Fixed the display of the tooltip for the module.

No idea how to solve the positioning of the module link for the breadcrumbs etc which are showing on the far right of the page

@brianteeman SCSS linter says for file templates/cassiopeia/scss/blocks/_form.scss, line 82: Expected "right" to come before "box-shadow" order/properties-order, see https://ci.joomla.org/joomla/joomla-cms/39355/1/20

I was waiting for it to finish the run

new update

@brianteeman I would have preferred the following SCSS:

a.jmenuedit {
  padding-right: .5rem;

  [dir="rtl"] & {
    padding-right: 0;
    padding-left: .5rem;
  }
}

That would not add a padding at the left hand side (when ltr).

But maybe that's a matter of taste.

As neither of us are frontend designers I suggest leaving it as is until they have any opinion or merge and accept that there may be future pr to tidy up the style

As neither of us are frontend designers I suggest leaving it as is until they have any opinion or merge and accept that there may be future pr to tidy up the style

For me that's ok. Will test again later. Let's see what others say.

A few issues are remaining, see red numbered marks in following screenshot:

1. The padding before the menu item edit icon, see my comment above about how I would fix it.
2. The position of the module edit button for horizontal menus, i.e. default menu layout in the header area or Cassiopeia's dropdown menu layout in the header area or at other places when horizontal orientation (module class menu-horizontal, as far as I remember).
3. The known problem with positioning of the module edit button for the breadcrumbs module mentioned above.

But without this PR here, there is also a problem with position of the module edit icon of horizontal menus, see red mark in following screenshot:

And for the breadcrumbs module and the search module in the header, you don't get the icon at all (if not very lucky maybe) without the PR.

So this PR is fine for me, all remaining issues can be fixed with (S)CSS.

We only should keep track of them so they are not forgotten.

I have tested this item ✅ successfully on d5ac463

Details about my test result see my previous comment.

2 and 3 are the same issue

Thanks for testing

I have tested this item ✅ successfully on d5ac463

RTC

RTL: dead fish.

.btn.jmodedit
should have an RTL equivalent with left: 10px; instead of right.
Also the icon should have a padding-left: 3px;

@infograf768 is correct that this needs changing for RTL but there is no longer any point in doing that in this PR as with bootstrap 5 we will change to use -start and -end so there will be no need for the separate RTL css but I can't write that until the bs5 stuff is merged so please go ahead and merge this as is and then we can handle all the bs5 css changes later

@brianteeman can you please solve the merge conflict I expect it's from the bs5 merge

@HLeithner done

merged it please take care of the rtl topic if needed.

Thanks - will look at rtl first thing tomorrow