@laoneo @dneukirchen @dgrammatiko @ciar4n it would be great if you guys do some review

@kasvith kind of hard to test because as @proman24 days it cant be tested because of joomla-projects/media-manager-improvement#577

Error on installing PR: The file marked for modification does not exist: package-lock.json

@franz-wohlkoenig just pushed a commit, that might solve the issue

@proman24 no change, same Error. Maybe it depends that Test using Patchtester didn't work in this Case too.

@franz-wohlkoenig This PR is bot passible to test on nightly builds because package-lock.json is excluded from builds

@proman24 Great work :) Check few comments I left. And, its good if you can create .es6.js version of js file.

Thank you @Anu1601CS for reviewing, I'll do the changes ASAP.

@dgrammatiko Changes in administrator/components/com_media/package-lock.json is correct ?

@proman24 Why changes occurs ? in administrator/components/com_media/package-lock.json

@Anu1601CS No idea why administrator/components/com_media/package-lock.json is changing, I have not used any NPM packages in the plugin.

@Anu1601CS No idea why administrator/components/com_media/package-lock.json is changing, I have not used any NPM packages in the plugin.

Then better to revert the changes you have not done!

I have tested this item ? unsuccessfully on 494e1d7

- Loading time is quite long

• Focus is not respected at resizing.
  But very very cool feature!!!

  ---

  _{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21775.}

Tested on alpha5.

Don't work for me neither. As long as I'm on the "focus" tab, "save" or "save and close" do not work.

@ka3media please mark your Test as unsuccessfully:

• open Issue Tracker
• Login with your github-Account
• Click on blue "Test this"-Button above Authors-Picture
• mark your Test as unsuccessfully
• hit "submit test result"

I have tested this item ? unsuccessfully on 494e1d7

Tested on alpha5.

Don't work for me neither. As long as I'm on the "focus" tab, "save" or "save and close" do not work.

@proman24 It's good if you can give small time here in your project PR :)

It would be really a great and professional feature!

Thanks for testing @coolcat-creations @ka3media. :-)
Feature is not functional now on this branch because of this issue.
But, you can check out the working feature at this repo master branch.
Would be correcting the issues with this PR once the template issue is resolved, cause as of now I am able to test the integrations.

impressive feature !!!! hope will be added !

Hi guys... this is probably one of most important features for let J4 be adopted by more "admin users", copywriters, website managers, etc.

Would be correcting the issues with this PR once the template issue is resolved, cause as of now I am able to test the integrations.

I hope @proman24 can check if the problem was solved ...

@proman24 what is the status of this PR? It's a great feature, I'm here to test for you.

@proman24 I cannot replicate joomla-projects/media-manager-improvement#577 which may mean it has been resolved somewhere.

@proman24 using patch tester I have added this patch and it does not show me the additional tab that you mention. As you have mentioned there is no requirement for NPM I will need further instructions. Thanks so much for this really nice addition to Joomla. Tag me and I will test it again when I have further information.

I have tested this item ? unsuccessfully on 494e1d7

Applying the patch did not bring this tab up. So test failed.

@uglyeoin thanks for testing this PR and verifying other issues too[which was affecting this PR]. I am looking into this PR and resolving the issues with it.

@proman24 you the man, I can't wait to test it again.

@uglyeoin just pushed some commits, can you please test the PR. Thanks!

@proman24 I applied the patch in patch tester and I did not see the tab in media manager when editing and image. Do I need to run NPM?

GOTO Administrator->System->Install->Discover

Install

• Adaptive image content plugin
• Media action focus

I did that, I found both and installed both. I still don't see the focus tab.

Am I looking in the right place? I went to content > media > hover an image and click the pencil

Ignore that I published the plugins now too

If I may make a suggestion. It is only possible to have the focus in a rectangle. Possibly because of the original shape of my image. One of the great features of the <picture> element is that you can change the art direction. Which may mean that on mobile for example I do not want a landscape rectangle but instead a portrait image. Does that make sense?

I have tested this item ✅ successfully on cfa97a1

It works but I have suggested an improvement. I have tested on the front end and the back end.

I don't know if this should be part of this pull request or not, I suspect not. But another great benefit of the <picture> element is that you can modern images, such as WebP with a fallback such as JPEG. I have no idea how you would achieve that with the media manager but in the long run I think that's the direction we should be taking. You can see an example we made here: https://dev.joomlalondon.co.uk/ (site isn't finished yet).

@proman24 great work, let me know when you need me to test again.

@proman24 is this ready to test?

@proman24 when I use this I can change the focus but it always needs to be at the same aspect ratio as the original. So if it's a landscape image it needs to remain landscape, but on a phone I would often want the image to be portrait. Is there a way to change that?

@uglyeoin plugin was working as expected, as tested on 8c2afdb. @Quy @brianteeman can you please review the code so that It can be merged.

@uglyeoin In SmartCrop.php in compute function, I am taking the required width as the parameter, and cropping the image according to their original aspect ratio, another parameter -- like aspect ratio can be passed in this function to satisfy this requirement. Also, some minor changes in the content plugin can be done to judge whether to show landscape / mobile cropped images.

on a 1st look you need to add the relative installation & update sql files for these plugins

on a 1st look you need to add the relative installation & update sql files for these plugins

There are few of my comments that are not addressed:

• The js files are in the wrong place media/com_media/js/edit-images.js
• with wrong ext .es6.js
• same for css...

But on a quick glance this is totally useless:

• The ux is not 2020 (upload the file, then go select the file apply the crop... WTF???), actually the ux is not even considered...
• Although it is built upon my code: https://github.com/ttc-freebies/plugin-responsive-images this is seriously not the ideal plugin for the core implementation of srcsets.
• The JSON file as a storage is a no-no from anyone that did anything that was supposed to sustain a load (some 10000 of files in this case)

In sort this is not FIT or either not production READY.

There you have it, someone said it...

@dgrammatiko thank you for your feedback. I accept you as an authoritative voice, but perhaps the voice could have been slightly gentler. Let's try to offer solutions as well as problems and be mindful that someone has put a lot of work and time into this.

I don't think the UX is perfect either, but I don't have a suggestion on how to improve it which is why I have not mentioned this. Do we have a UX team?

I personally believe we should be using <picture> not srcsets as <picture> can incorporate srcsets as well as cropping and also offering other image formats such as webp with jpeg as a fallback.

I don't think we can use the current system - one image to serve all viewports - so we do need a solution if we are to be a modern CMS.

I personally believe we should be using not srcsets as can incorporate srcsets as well as cropping and also offering other image formats such as webp with jpeg as a fallback.

That's implementation detail, yes picture elements are better nowadays

I don't think we can use the current system - one image to serve all viewports - so we do need a solution if we are to be a modern CMS.

Yes the CMS needs a way to have thumbnails or sets of images per given breakpoints (srcsets). What the CMS doesn't have to provide as it's a way to niche solution (very few will actually either use it) is a cropped thumbs per breakpoint (eg this is providing only one cropped version, someone could argue why not have an individually cropped image per breakpoint?). See my point?

I don't think the UX is perfect either, but I don't have a suggestion on how to improve it which is why I have not mentioned this.

Well it's easy: instead of waiting the user to come back for this functionality provide it whenever there is an upload. Simple as that...

but perhaps the voice could have been slightly gentler.

Sorry not sorry for expressing honest opinion here. Truth hurts sometimes...

Sorry not sorry for expressing honest opinion here. Truth hurts sometimes...
I didn't say don't express an opinion, they are vital for driving the project forwards. I just implied it could be expressed in a more considerate way. Your original post reads as this is dead wood and should not be worked on.

Simply by suggesting in more detail what the issues are and how they may be overcome is a lot more objective.

By saying "if this was offered on file upload it would be a better implementation" it would be taken less badly than "sorry this is totally useless". Someone spent time on this, we should be grateful, even if we don't use it. If there are better solutions, suggest them as options, an objective way.

Your original post reads as this is dead wood and should not be worked on.

Not "as if this is dead", I say it very clearly!!!

Let me explain:

• Architecturally: it fails, not fixable unless rewritten
• UX: it fails, not fixable unless rewritten
• A11y: it fails, removes blindly the alt attributes (actually will remove any attributes except the src)...

It's pretty straight forward: this is not for Prime time...

It would be better to say things objectively is my only point. If code doesn't fit the requirements the project should not be obliged to accept it. By explaining what would make it acceptable the original author or someone else may decide to take the job on. Without that clarity, strong statements such as "this is totally useless" demoralise people who have taken time to contribute.

It is still currently better than nothing, albeit I'm not a fan of accepting things for that reason. So many would say it is not totally useless, or even useless.

Your "do it on upload" solution brings with it the problem of needing to change media queries or edit crops when a template changes (or for any other reason), so we still need another way to edit things if we take that route forward.

UX should not be the opinion of one person, it should be a data driven, testable process.

In any case, people have written code & documentation, we can still be grateful to the original author for his work (thank you @proman24 ). To avoid wasting people's time, we should get an overriding decision on how to move forwards. I don't know who is in charge of that. It would also be good to define tasks better up front rather than afterwards if that is at all possible.

It is still currently better than nothing, albeit I'm not a fan of accepting things for that reason. So many would say it is not totally useless, or even useless.

And this is how Joomla end it up where it is now, by accepting code that should have been rejected

FWIW I've helped the author of this PR to complete his GSOC task but I wasn't his mentor neither I was involved in the architectural, design or UX decisions. In sort there are things that could be salvaged and then there are things that would take far less time to be done from clean sheet. This in my humble opinion is falling to the second category, it's far easier to write a proper thingy than try to make this work properly...

My 2c and I'm off this thread...

The ux is not 2020 (upload the file, then go select the file apply the crop... WTF???), actually the ux is not even considered...

Without giving a way to users to select the focus area how are we taking in the focus? I don't think of any other way. Also, this is not mandatory for all the images only those which author required, just like other tabs in the modal, crop/rotate.

Although it is built upon my code: https://github.com/ttc-freebies/plugin-responsive-images

Totaly no clue about this thing. And I strongly disagree with this false accusation of plagiarism, anyways.

The JSON file as storage is a no-no from anyone that did anything that was supposed to sustain a load (some 10000 of files in this case)

At the time of development, the media-manager was not using any database, so upon discussion, we decided to go with a JSON store. So that after this thing is functional, DB class can be written in the near future.

Well it's easy: instead of waiting the user to come back for this functionality provide it whenever there is an upload. Simple as that...

The main idea was to provide the user with a way to set the focus of the image, and process image according to the focus area

Yes the CMS needs a way to have thumbnails or sets of images per given breakpoints (srcsets). What the CMS doesn't have to provide as it's a way to niche solution (very few will actually either use it) is a cropped thumbs per breakpoint (eg this is providing only one cropped version, someone could argue why not have an individually cropped image per breakpoint?). See my point?

I not getting what are you trying to say here, In this plugin, we are cropping the image for individual breakpoints around the focus defined by the user.

Everything was very well discussed and iteratively developed, under the mentorship of @nibra @kasvith, So, giving conclusive statement like the whole design of the plugin is wrong, would not be fair-- it's far easier to write a proper thingy than try to make this work properly, a quick glance this is totally useless,this is not FIT or either not production READY . I believe that there might be some holes in the plugin which could be filled up but claiming it to be wholly wrong doesn't seem to be worthy for the time and efforts put in by me and the mentors.

@dgrammatiko after your comment on the first evaluation blog, I messaged you directly for knowing where is this thing going wrong. But that was a very short and discontinued discussion.

There were some delays in testing and getting the PR merged in 2018 because of some other underlying issue which I stated in the description of the PR. But I am more than interested in getting this PR merged in Joomla not only because I have spent more than 4 months in the development of this plugin, but also I believe that this is a really good add-on to Joomla's core.

It would be great if someone can help me in figuring out what exactly are the changes needs to be done (rather than saying architecturally wrong) and what can be done in this plugin for making it production-ready.

Also, before continuing with the development we should get the vote weather to continue developing this plugin the way it is defined in this PR. Or there is some better way to achieve the objective. If yes, then it would be a good idea to close this PR.

@laoneo @brianteeman @ciar4n @Quy @nibra @kasvith @dneukirchen

@wilsonge Any use in continuing development of this feature?

Yes I definitely would like to see a version of this make it in - clearly needs some work though. So as a starting point would be good to merge in latest 4.0-dev and move the media folders into the build/media_source directory. I'm just spinning this up locally now to do some testing and figure out the next steps beyond that.

As @proman24 mentioned the plugin uses JSON store because NMM did not use any database at that time. But the plugin itself does not care what is the storage system for these data. It can be MySQL, Firebase, Postgres whatever we want if we have a need in future. Also IMHO since there arent any frequent updates JSON still fits pretty fine there(we can introduce a new storage system if we getting issues after releasing to beta).

As @nibra and I can clearly say, @proman24 has put a great effort into this project. Let's not waste his effort.

• If this is not good in UI/UX it's better to do a proper redesign and make this plugin work as UI/UX report suggests.
• About A11y, I'm sure we can add that somehow(sorry I don't remember much coz it was like 2yrs ago).

I would suggest resolving this matter and move forward, and not to close PR.

please don't enable this by default. As a photographer and sometime site dev I dang sure don't want some piece of software changing images I spent time to create.

@N6REJ Nothing is done to your image unless you tell it to be done by setting a focus.

I am moving this to 4.1, we are in beta and this is a new feature with need some more work.

@proman24 is there any interest to work through the feedback and make this PR mergable?

This pull request has automatically rebased to 4.2-dev.

This pull requests has been automatically converted to the PSR-12 coding standard.

closed, because has been taken over by another person #38264