Why do you insist on abusing URLs?

Why do you insist on abusing URLs?

There is no abuse of the final URL. Also what is the problem with the URL? I'm using it to pass information that otherwise I have to somehow calculate in runtime which will defeat the purpose of any perf gains...

Really would be simpler and cleaner to add width/height fields for intro/fulltext images.

Really would be simpler and cleaner to add width/height fields for intro/fulltext images.

Nope, It requires 2 additional fields and 2 additional sql columns and will break any existing extension. Also it will be a mess trying to sync 3 fields in the JS.

We don't need to add any columns. All image related data is JSON encoded and stored in a single column.

We don't need to add any columns. All image related data is JSON encoded and stored in a single column.

You're welcome to do a PR with that. BTW the idea is that ALL images using the form field media should have the ability to use those attributes (system wide change). Curious to see how your approach will solve that...

I have installed the package from the packager section using there com_content (+tinymce) I get the console error message

Also it seems the new "alternative text" stuff is missing?

Or I'm doing something wrong on my side?

@zero-24 I can't replicate with the branch code (tested with Safari, FF, Chrome), maybe something broken in the package?

hmm will try again and come back to you :)

Update test on MySQLi and PostgreSQL is ok. Now am doing new installation tests for both database types.

Sorry, wrong PR.

Alt text/lazy load checkbox should be hidden if a folder is selected. To reproduce, click an image, then click a folder.

Click several images consecutively reduces the viewable area.

Click several images consecutively reduces the viewable area.

This was supposed to be fixed with 1b9b603

Alt text/lazy load checkbox should be hidden if a folder is selected

Selecting a folder is not firing an event thus it cannot be handled. FWIW the alt text input + lazyload checkbox are not part of the media manager (not the component's responsibility how an image will be used in any extension anyway). If this is show stopper let's fix it here either wise open an issue after this PR is merged (if ever). The reason is that it requires some more tweaking in the media manager and I don't won't to do it here, the PR is already touching a lot of files as is...

This was supposed to be fixed with 1b9b603

I installed with the latest prebuilt package from here so it should have been included/fixed, but it is still happening.

https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/30784/downloads/35808/

I installed with the latest prebuilt package

It was my fault I tested the code locally but manage to push something different...

hmm will try again and come back to you :)

Just re tested. It seems you have to "click" the image and it is not egnoth to just "select the image" could that might be a different event?

It seems you have to "click" the image and it is not egnoth to just "select the image"

That seems to be a media manager bug, or to be more accurate here the selected box should be controlled by js not css driven

That seems to be a media manager bug, or to be more accurate here the selected box should be controlled by js not css driven

Ok can that be fixed here or better with the other PR for the media manager as mention above?

Ok can that be fixed here or better with the other PR for the media manager as mention above?

I would rather tackle this in another PR. The only things touched in Media manager here were the missing arguments for the event, changing the selection behaviour has a bigger scope (css + Vue) and I guess it needs it's own testing procedure. But I'm ok doing it here if people are willing to aggressively test media manager

I have tested this item ✅ successfully on ac18bf2

I would rather tackle this in another PR. The only things touched in Media manager here were the missing arguments for the event, changing the selection behaviour has a bigger scope (css + Vue) and I guess it needs it's own testing procedure. But I'm ok doing it here if people are willing to aggressively test media manager

I'm happy to do so. :)

@zero-24 @Quy the changes are not that bad:

in actions.js add

/**
 * Dispatch a window event expected in the media image select script
 *
 * @param detail {object}  The item details
 */
const fireWindowEvent = (detail) => {
  window.parent.document.dispatchEvent(
    new CustomEvent(
      'onMediaFileSelected',
      {
        bubbles: true,
        cancelable: false,
        detail: detail,
      }
    )
  );
}

and change export const toggleBrowserItemSelect = (context, payload) => { to

/**
 * Toggle the selection state of an item
 * @param context
 * @param payload
 */
export const toggleBrowserItemSelect = (context, payload) => {
    const item = payload;
    const isSelected = context.state.selectedItems.some(selected => selected.path === item.path);

    if (!isSelected) {
        // Unselect all other selected items, if the shift key was not pressed during the click event
        if (!(event.shiftKey || event.keyCode === 13)) {
          context.commit(types.UNSELECT_ALL_BROWSER_ITEMS);
        }

        context.commit(types.SELECT_BROWSER_ITEM, item);

        if (item.path && item.type === 'file') {
          fireWindowEvent({
            path: item.path,
            thumb: item.thumb,
            fileType: item.mime_type ? item.mime_type : false,
            extension: item.extension ? item.extension : false,
            width: item.width ? item.width : 0,
            height: item.height ? item.height : 0,
          });
        } else {
          fireWindowEvent({});
        }
    } else {
      context.commit(types.UNSELECT_BROWSER_ITEM, item);
      fireWindowEvent({});
    }

  // If more than one item was selected and the user clicks again on the selected item,
  // he most probably wants to unselect all other items.
  if (context.state.selectedItems.length > 1) {
    context.commit(types.UNSELECT_ALL_BROWSER_ITEMS);
    context.commit(types.SELECT_BROWSER_ITEM, item);
  }
};

in browser,vue change unselectAllBrowserItems(event) { to

            /* Unselect all browser items */
            unselectAllBrowserItems(event) {
                const clickedDelete = (event.target.id !== undefined && event.target.id === 'mediaDelete') ? true : false;
                const notClickedBrowserItems = (this.$refs.browserItems && !this.$refs.browserItems.contains(event.target)) || event.target === this.$refs.browserItems;
                const notClickedInfobar = this.$refs.infobar !== undefined && !this.$refs.infobar.$el.contains(event.target);
                const clickedOutside = notClickedBrowserItems && notClickedInfobar && !clickedDelete;
                if (clickedOutside) {
                    this.$store.commit(types.UNSELECT_ALL_BROWSER_ITEMS);

                    window.parent.document.dispatchEvent(
                        new CustomEvent(
                            'onMediaFileSelected',
                            {
                                "bubbles": true,
                                "cancelable": false,
                                "detail": {}
                            }
                        )
                    );
                }
            },

and lastly in item.js change handleClick(event) { to

        /**
         * Handle the click event
         * @param event
         */
        handleClick(event) {
          this.$store.dispatch('toggleBrowserItemSelect', this.item);
        },

But let's fix this in another PR

But let's fix this in another PR

Fine for me ?

If the alt text The "quote" test contains a double quote, then only the text up to it is saved.

CMS Content > Image
<img src="images/joomla_black.png" alt="The " width="225" height="50" loading="lazy" />

Insert > Image
<img src="images/joomla_black.png" alt="The &quot;quote&quot; test" width="225" height="50" />

If the alt text The "quote" test contains a double quote, then only the text up to it is saved.

That also wouldn't work in J3.x:

If it's only the " that needs escaping then the fix is really easy:

    altInputFn(e) {
       this.setAttribute('alt-value', e.target.value.replace(/"/g, '"'));
     }

Edit: fixed with 92294f6

To test it open the browser console and paste

'Some "text" and some random quote: "'.replace(/"/g, '"');

Would get

Or test it with the D/L package

@Quy can you please re test this one so it can moved to RTC and have the maintainers decide if this is acceptable or not?

and have the maintainers decide if this is acceptable or not?

once tested i can hit merge here. ?

There are a few todos to translate strings. Do in a separate PR?

Also there is Confirm text that I couldn’t get it to reproduce in a test. Please provide steps.

There are a few todos to translate strings

The strings are for TinyMCE, should be done in the crowdin

Also there is Confirm text that I couldn’t get it to reproduce in a test.

There is no Confirm anymore

The strings are for TinyMCE, should be done in the crowdin

sure but they have to go via (J)Text right? Crowdin only takes the ini files for translation

sure but they have to go via (J)Text right? Crowdin only takes the ini files for translation

AFAIR Joomla has it's own translation for tinyMCE: https://github.com/joomla/joomla-cms/tree/4.0-dev/build/media_source/vendor/tinymce/langs

BTW there should be an npm package for these...

AFAIR Joomla has it's own translation for tinyMCE: https://github.com/joomla/joomla-cms/tree/4.0-dev/build/media_source/vendor/tinymce/langs

Ok so the mention stings needs to be added to that original file and than pushed to the translators right?

Ok so the mention stings needs to be added to that original file and than pushed to the translators right?

Im not sure how these files are generated, I think there is a repo somewhere

Im not sure how these files are generated, I think there is a repo somewhere

I'm not aware of any maybe @infograf768 or @Bakual can help here?

The tinyMCE translations are the official translations from tinyMCE I think it's on Transifex.
IIRC the TTs provide them to us and we manually update them when needed.

Ok so what is the process to add new language strings to that project. I dont think we can push them via tinyMCE's Transifix :D

I haven't looked at this PR in detail. It sounds as the string isn't used by the editor itself? So those files would be the wrong place anyway. They can be loaded by our plugin with the INI file like we do in other places of the CMS.

Thanks @Bakual

@dgrammatiko can you please update the code for that and load the strings from the ini files / via (J)Text?

@dgrammatiko can you please update the code for that and load the strings from the ini files / via (J)Text?

I'll do that in a bit. But if Joomla is using the default TinyMCE translations why do we have the files in the build folder instead of fetching them from their repo? I think something is of here. I still remember ppl telling me to use OUR files instead of the official ones

@zero-24 done

@zero-24 done

Thanks!

I'll do that in a bit. But if Joomla is using the default TinyMCE translations why do we have the files in the build folder instead of fetching them from their repo? I think something is of here. I still remember ppl telling me to use OUR files instead of the official ones

TBH I dont know but I would say this is out of scope for that PR, can you open an issue for that? Do you have a suggestion where should it be better placed?

I have tested this item ✅ successfully on ac18bf2

@dgrammatiko can you please update the code for that and load the strings from the ini files / via (J)Text?

I'll do that in a bit. But if Joomla is using the default TinyMCE translations why do we have the files in the build folder instead of fetching them from their repo? I think something is of here. I still remember ppl telling me to use OUR files instead of the official ones

Historically that was because the TT said that the quality was very bad

@wilsonge can we have a verdict on this one?

@dgrammatiko I'm sorry that it took me that long to come back to you on this. It seems there are conflicts can you take a look? Will now test with the latest build from drone and log my results here.

Ah seems my message and your merge just crossed. Thanks! :D

It's not ready yet, some dates are out of sync, gimme 5 mins

Pfff, this is ready for testing again...

Hm so from my perspective this functionally works. Two things. One major the other less so:

Major issue: Fulltext images no longer scale to fit their text area

Here's a before and after patch (this is because we're hardcoding the width/height attributes). This was a screenshot of my large monitor - obviously however this would cause issues if i had a more regular size image but wanted to render it on mobile.

More minor thing (not sure if this is a bug - or just something we need to document now): It's now impossible to insert an image without an alt text when using the media manager modal/drag n' drop. It always gets an empty alt attribute (or correctly the one inserted) - before obviously it was impossible to have an alt text

Major issue: Fulltext images no longer scale to fit their text area

As I stated in the documentation section of the description:

Provide some guidelines for styling the lazy loaded images, basically explain how to use the css property object-fit: scale-down, cover, fill, etc https://caniuse.com/object-fit to get the same responsive behaviour as without defined width,height

Basically this can be fixed with a one line of css (eg object-fit: scale-down targeting the specific class). The reason I din't introduced this was because people might have different tastes for this (the behaviour is quite different between fill, scale-down etc)

before obviously it was impossible to have an alt text

Actually there was a request from the a11y people to have always an empty alt tag, (that was before this PR introduces the ability to insert some alt text). So all I did was following their request. ref: #28928 (comment)

EDIT: Probably we can do something like:

[loading="lazy"] {
  object-fit: scale-down;
}

If someone can confirm that the specificity is ok (eg a class selector will override this). @ciar4n ?

@wilsonge there's an easier solution:

[loading="lazy"] {
  width: 100%;
  height: auto;
}

Live demo: https://codepen.io/dgrammatiko/pen/WNxwYRR

Actually there was a request from the a11y people to have always an empty alt tag,

I think there is some confusion here. An image should always have an alt description unless it is purely decorative. Only if it is purely decorative should you use alt="". so almost always an image should have an alt description.

The problem with defaulting to "" if there is no alt description entered is that this mistake of not having a description is now hidden from any accessibility checker and the user will think erroneously that they are creating accessible content.

It should be a conscious decision to have an alt empty (alt="") and not a default.

In some other CMS this is done by having a checkbox like in this mockup

It should be a conscious decision to have an alt empty (alt="") and not a default.

@brianteeman the issue raised was about the drag n drop in tinyMCE and when I was reworking the broken code #28928 (comment) @chmst asked me to default to empty tag. That was before this PR. With this PR we have a modal asking for some alt text (which can be left empty and thus we end up with an empty alt tag). If you want me to add one more check box that will remove also the empty tag I can do that, although I don't think it's advisable...

I do not agree that we should default to an empty tag ie alt="" for the reasons I stated before

I do not agree that we should default to an empty tag ie alt="" for the reasons I stated before

But we are not, this was the case before this PR. The empty tag is getting rendered only if a user will not supply some alt text. I think my bad explanation was caused the confusion here...

we are defaulting to alt="" - I checked
A user usually does not supply some text because they are lazy or do not realise the importance of it.

If they then run an accessibility check on their site they will not be notified about a missing alt text on an image so they will never learn to improve and add the text

Using alt="" should be a conscious decision and not a default

Using alt="" should be a conscious decision and not a default

It seems that the a11y people need to come up with one solid answer here, I can not satisfy different and contradictory requirements. Anyways, if this is an issue please provide a workflow (that people will not blame me for introducing it) and I'll code it

Well I have suggested a workflow in the image above.

Well I have suggested a workflow in the image above.

We are talking about different things then. The issue George raised was about these 2 cases:

So basically George me and Christiane we are talking about content images and you are talking about the media field...

No that was just an example. The point about the alt="" being a bad default is valid everywhere

In other words this line of code is wrong
const altValue = alt="${dialogData.altText || ''}";

In other words this line of code is wrong

This will only append alt tag if there is some text

const altValue = `${dialogData.altText ? 'alt="${dialogData.altText}"' : ''`;

But this will not enforce alt tags and realistically will keep the majority of sites produced with Joomla to fail any a11y test ( we all know that people don't write alt texts)

It forces an empty alt description which is wrong. They should fail. We want them to fail. Only then will people start to make their sites accessible and enter alt text

It forces an empty alt description which is wrong. They should fail. We want them to fail. Only then will people start to make their sites accessible and enter alt text

I will not implement anything more here, you (or the a11y team) are welcome to refactor the code to fit your needs. Putting myself between arguing concepts never had a good outcome for me...

[loading="lazy"] {
  width: 100%;
  height: auto;
}

hmm this seems to overwrite the inline width and heigth settings?

before that CSS

after that CSS

[loading="lazy"] {
object-fit: scale-down;
}

with that it looks better but for some reasone there is now an empty gap?

I will not implement anything more here, you (or the a11y team) are welcome to refactor the code to fit your needs. Putting myself between arguing concepts never had a good outcome for me...

Agree. As this is an topic before this PR here and also beyond this PR I would suggest to branch that issue out to its own issue so we can discuss all sites and than implement one solution for all cases.

Agree. As this is an topic before this PR here and also beyond this PR I would suggest to branch that issue out to its own issue so we can discuss all sites and than implement one solution for all cases.

Sorry but that is not true. This is a change in behaviour introduced in this PR

Sorry but that is not true. This is a change in behaviour introduced in this PR

Well agree but it introduced the same behavior than we have in other places here too (when there is nothing we add alt=""). To change that i would suggest to do a dedicated issue so this can be adressed at all places.

That are both screenshots without this PR here applied

Those are all correct uses.
This is NEW behaviour being added

Those are all correct uses.

You disagree your self, by suggesting this checkbox a few posts above right? #30784 (comment)

This is NEW behaviour being added

Yes but implmented exactly in the same way than the image / media picker works right now. No alt text = alt=""

So my suggestion is when that behavior should be changed (e.g. adding that tick box as in your scteenshot: #30784 (comment)) than that should be done in a dedicated Issue / PR in my opinion.

Our bad that neither I nor the accessibility team spotted that error. Its a good example of what I am saying about the error hiding the problem with an accessibility check and the only way to find it is to read all the code but PLEASE dont introduce another one.

Our bad that neither I nor the accessibility team spotted that error. Its a good example of what I am saying about the error hiding the problem with an accessibility check and the only way to find it is to read all the code but PLEASE dont introduce another one.

It is there since 3.x? https://github.com/joomla/joomla-cms/blob/staging/layouts/joomla/content/full_image.php#L21

So can we please agree that this is a general problem to be discussed and fixed but nothing that should hold back this PR (lazyloading) as this here does exactly that what we do in all other places of the current code base too.

if only we had a group of people who checked accessibility

if only we had a group of people who checked accessibility

In this specific case it was even a member of the accessibility team who suggested that change, right? So they even have checked this PR. I can not comment on what is the right solution but please when that is an issue that should be fixed lets move this discussion to a dedicated issue, involve that team and please unblock this PR.

Update:

• with commit 044c91b I reverted inserting an empty alt tag is no text provided.

• About the insertion or not or whatever, please create an issue, I'm not willing to solve this here, just because one person is objecting...

• About the styling, @zero-24 the selector definitely shouldn't be global, the example given code above should be way more specific on what images will be targeting (intro, fulltext, content images [inside the content area], etc). This needs to be solved in the template and as I stated before I don't won't to preoccupy people with any solution, I just provided some possible ways to tackle this

• @wilsonge this PR already revealed some mishaps in the Media manager emitted events, some styling issues and some a11y. Realistically these can not be tackled in one PR. I already provided the code to fix MM and have more code about the media field but all that is stack awaiting this one to be merged. So can we either bite the bullet and then start fixing all the remaining issues as follow ups or close this?

@dgrammatiko thank you
I already created the issue this morning #31138

@wilsonge @zero-24 is there any decision on this one?

@wilsonge @zero-24 is there any decision on this one?

You have my full support for this PR to get it merged. ?

I think this is fine. I don't love the concept of having the query params in the saved item - I'd rather have it saved directly into the JSON blob in the DB - but modifying JForm to validate this sounds problematic - so I guess it will have to do. Can we get a couple of tests here and I'm happy for this to be merged - as we're fixing several issues in the existing lazy loading implementation

I have tested this item ✅ successfully on 182a275

Logging my test from above here. Thanks a lot @dgrammatiko ?

but modifying JForm to validate this sounds problematic

It's not problematic. Only need to add width/height fields to the form and get their IDs in media field.

It's not problematic. Only need to add width/height fields to the form and get their IDs in media field.

Well than lets please fix this in a follow up PR so we get the baseline working and than can improve the other parts too.

It's not problematic. Only need to add width/height fields to the form and get their IDs in media field.

Unless I'm totally wrong I think it's kinda hard to have one form field to spawn different values (eg value for the srcand also values for width and height). If you have a solution that the hidden fields for width and height are part of the media field I'm happy to include it here

Why does this have to be one field? And why would width/height fields be hidden?

And why would width/height fields be hidden?

Their value aren't up to the user to decide, it's coming from the actual image dimensions. Also adding more inputs increases frustration for users and destroys UX, especially if those inputs are just there to pass data (that is automatically filled) to the PHP...

Why does this have to be one field?

According to @wilsonge 's comment, to which I totally agree:

I'd rather have it saved directly into the JSON blob in the DB

Actually FWIW the media field is totally wrong. It should be a single module (eg the field has to control all the related inputs (alt, position, etc) that saves a JSON blob as George suggests but for me I think it should be either broken up to 4 fields (image, audio, video, document) to reflect all the possible media that the media manager can hold or as one field (probably less bloated) with a possible exported JSON blob like:

  {
    "type": "image", // or audio, video, document
    "extension": "webp",
    "src": "path/to/file.webp",
    "attributes": {
      "width": 600,
      "height": 300,
      "alt": "some alt text",
      "caption": "some caption text",
      // Attributes for audio/video/docs can be populated here
      "poster": "path/to/poster.jpg"
    }
  }

Of course for the front end this data needs to be coupled with the relevant jLayouts ( eg parent jLayout mediaField.php will switch to the correct children layout depending on the type)
Only problem with this approach is that is a really hard B/C break (although we can create a new field named, eg mediaObject) and deprecate the media field, although this realistically won't motivate the 3rd PD to embrace lazyloading. In sort there are many possible ways forward but the idea to have sparse, loosely related inputs that are up to 3rd PD to implement is not for the best interest of the project, it's totally against any theory of modularity and it's definitely not the way to implemented in 2020.

If we're adding image dimensions, there is no reason not to allow users to set them. Pre-populating the fields is fine. Forcing some arbitrary values is not.

We already store images as JSON in most places. So adding additional fields is not a problem and is in line with what we already have. This would be the cleanest, mostly B/C safe solution for the time being.

Something similar to an enhanced media already exists in Joomla\CMS\Form\Field\AccessiblemediaField, although not a great technical example. The problem with something like this is it would likely be too opinionated. For instance, your given example doesn't contain a class attribute which we do need in some places. And it has a caption which we may not need in other places. And making available attributes configurable just adds more complexity for the devs.

If we're adding image dimensions, there is no reason not to allow users to set them.

In the same pattern then someone can select a file test.jpg and then could change the extension to .png and that file might not exist and you get a broken field. The dimensions are not editable from the user, their data is extracted from the actual image size. Allowing this to be manipulated from users is totally stupid, adds more frustration, destroys UX and finally could have a disastrous effect if misused.

We already store images as JSON in most places. So adding additional fields is not a problem and is in line with what we already have. This would be the cleanest, mostly B/C safe solution for the time being.

What exists in the db right now is something that designed many many years ago. Keep building on top of a wrong foundation is not a good call. Just a reminder the JSON that exists in the db fields comes from an era that mysql didn't have support for json columns and most importantly Joomla didn't have Custom Fields...

For instance, your given example doesn't contain a class attribute which we do need in some places. And it has a caption which we may not need in other places.

Disagree. Class is something I omitted (I did omit a lot of the possible data there because that was a quick example not an exhaustive list) but this and many others could/should be there. Also if the field is cleverly coded should allow a great degree of customisability.

And making available attributes configurable just adds more complexity for the devs.

Safe defaults and documentation is my answer here.

Anyways I think that whatever the next steps are this is good enough for the B/C part of the media field. If a new concept comes it can coexist and the media field can be deprecated. But all this conversation, I think it needs to move away from this issue. The current implementation brings lazyloading to the current media field (which is opt out, for those that want to) in a way that is globally available. And to close this I urge you to search for type="media" and then check how many of these fields have a JSON representation in the db. With your idea to add sparse, unrelated fields for the dimensions we have to fix a lot of these fields. So, basically what you're suggesting is not B/C (in the sense that it doesn't work without further work) and needs a lot of work to be implemented and finally, doesn't really motivate 3rd PD to move there (nobody will go and add random fields in their forms, that's a fact)

In the same pattern then someone can select a file test.jpg and then could change the extension to .png and that file might not exist and you get a broken field. The dimensions are not editable from the user, their data is extracted from the actual image size. Allowing this to be manipulated from users is totally stupid, adds more frustration, destroys UX and finally could have a disastrous effect if misused.

All of this is wrong. I'm just going to repeat myself. Pre-populating values is fine, forcing them is not. Users may use dimension attributes to scale images or omit them altogether. This is not up to you to decide.

Keep building on top of a wrong foundation is not a good call.

This is better than introducing a new, out of place data structure. The upgrade path to proposed JSON structure is simpler if we keep using what we have now and the code is more performant. By adding attributes to URLs you'd have to both map existing fields and keep parsing URLs, unless someone decides that we should update user data to match new structure on upgrade to major version.

So, basically what you're suggesting is not B/C (in the sense that it doesn't work without further work)

That's not what B/C means at all. Having to add something to make a new feature work is not a B/C break. B/C here concerns existing user data. That said, adding new fields would be B/C safe in most places. Admittedly, this PR as it stands is mostly B/C too. But, in your own words, building on top of a wrong foundation is not a good call.

@dgrammatiko
also #31319 is a release blocker as alt, float and caption are missing

also #31319 is a release blocker as alt, float and caption are missing

This PR solves the ALT part

@wilsonge any decision on this one?

Yeah I still don't love it but i think it is a step in the right direction. FWIW i love the look of that generic json blob you posted.

Kinda agree with sharky we probably need to allow users to prepopulate the width/heights so they aren't maxed out. But I don't think that's a massive addon to this PR either.

Massive thanks to @dgrammatiko and @wilsonge here ?

Yeah I still don't love it but i think it is a step in the right direction

Well, I would also be happier with a json blob but we had a text field to play with. Maybe someone smarter than me could do the PHP part to allow json blob or fall back to the url text

Kinda agree with sharky we probably need to allow users to prepopulate the width/heights so they aren't maxed out

I disagree but I could implement this in a PR (we still miss class, caption, the new checkbox Brian added etc for parity with v3 and the other fields)

@zero-24 you're part of this the code for the custom fields was yours!!! Also not to mention that you were the one that triggered all these change, so big thanks

Also a big thank you to everybody involved here or any of the previous PRs

this PR already revealed some mishaps in the Media manager emitted events, some styling issues and some a11y.

For reference PRs #31427 and #31425 complete the remaining todos and #31667 fixes the display of the lazy loaded images for Cassiopeia

One extra bit here, the video Improving The Page Loading Experience To Reduce Layout Shift by Jen Simmons @jensimmons (webkit) https://www.youtube.com/watch?v=YM3KszYmn58 I've already included the slides but maybe video is more friendly

@dgrammatiko
Could not find the use of Text::script('JFIELD_MEDIA_CONFIRM_TEXT');

@infograf768 good catch, that's redundant

This thing of adding a query string joomla_image_width=225&joomla_image_height=50 to all images across the whole CMS is obscene. I wonder how it is possible that this thing got merged. @dgrammatiko honestly i hope that you will stop soon to contribute to Joomla, i've no words for you.

@joeforjoomla what's wrong with the added params in the URL?

honestly i hope that you will stop soon to contribute to Joomla, i've no words for you.

@joeforjoomla Regardless if your points are right or wrong, but this kind of personal attack is not right.

@joeforjoomla personal attacks are right if they come by @richard67.

@gostn Where have I attacked you or someone else personally? Can you show me that?

I understand you can't remember because you decided, what you wrote was not a personal attack.

I am now closing this to prevent further trolling and personal attacks.