Nice, would be cool to default it with the existing values, so we can remove them when not needed.

Nice, would be cool to default it with the existing values, so we can remove them when not needed.

@laoneo that has been taken care of.

I have tested this item ✅ successfully on 65fcf2e

Created a new aspect ratio and everything seems to work.

I have tested this item ✅ successfully on 65fcf2e

Testes this, calculated my own crop ratio. copied it and added it to the bottom. Cropped an image using this new crop. All seems to work fine.

RTC

I have tested this item ✅ successfully on 8d18b13

Thank you for fixing those errors

I have tested this item ✅ successfully on d79e1f7

Test by code review

Sadly, @hans2103 when applying Download package on Joomla 6.1 Nightly downloaded 30 minutes ago on PHP 8.4.15, I get:

An error has occurred.
0 Error loading form file

(changed to PHP 8.3 and same result ;()

Testing process:

Fresh Nightly install,
Sample Blog Data,
French language file,
Multilanguage Data,
Regular Labs Cache Cleaner
Joomla Patch Tester 5.0.0

Checked BEFORE condition of PR which were a match
Download PR package and then went to check Media - Crop plugin ;(

@hans2103 System tests would need some adjustment for this PR.

See https://github.com/joomla/joomla-cms/actions/runs/19510792565/job/55849853256?pr=46421 :

Running:  install/Installation.cy.js                                                    (1 of 148)

  Install Joomla
    1) "after each" hook for "Install Joomla"

  0 passing (14s)
  1 failing

  1) Install Joomla
       "after each" hook for "Install Joomla":
     CypressError: `cy.task('checkForLogs')` failed with the following error:

> [Wed Nov 19 17:43:33.751743 2025] [php:warn] [pid 263:tid 263] [client 127.0.0.1:47700] PHP Warning:  simplexml_load_file(): /tests/www/cmaria/plugins/media-action/crop/crop.xml:72: parser error : Opening and ending tag mismatch: fieldset line 25 and field in /tests/www/cmaria/libraries/src/Installer/Installer.php on line 2116, referer: https://localhost/cmaria/installation/index.php

And later those fail:

plugins/media-action/crop/CropPlugin.cy.js
plugins/media-action/resize/ResizePlugin.cy.js
plugins/media-action/rotate/RotatePlugin.cy.js

The system tests are correct!

@exlemor you are correct. The XML is wrong as identified by the system tests

Correction; the closing tag on line 39 is wrong it seems. although that doesn't fix the form completely if i change only that. Could be my install now but the calculator is broken atm. it just shows as a text field.

@hans2103 System tests would need some adjustment for this PR.

See https://github.com/joomla/joomla-cms/actions/runs/19510792565/job/55849853256?pr=46421 :

Running:  install/Installation.cy.js                                                    (1 of 148)

  Install Joomla
    1) "after each" hook for "Install Joomla"

  0 passing (14s)
  1 failing

  1) Install Joomla
       "after each" hook for "Install Joomla":
     CypressError: `cy.task('checkForLogs')` failed with the following error:

> [Wed Nov 19 17:43:33.751743 2025] [php:warn] [pid 263:tid 263] [client 127.0.0.1:47700] PHP Warning:  simplexml_load_file(): /tests/www/cmaria/plugins/media-action/crop/crop.xml:72: parser error : Opening and ending tag mismatch: fieldset line 25 and field in /tests/www/cmaria/libraries/src/Installer/Installer.php on line 2116, referer: https://localhost/cmaria/installation/index.php

And later those fail:

plugins/media-action/crop/CropPlugin.cy.js
plugins/media-action/resize/ResizePlugin.cy.js
plugins/media-action/rotate/RotatePlugin.cy.js

the issue was caused by commit d79e1f7
I have reverted it.

Sorry I must have been incorrect. Clearly when there is a subform you dont close the field as I had assumed

When in dark mode you can never see the calculated value and the button is hard to see

I think this is caused by an already reported bug with the dark mode css when used in alerts

The Calculated value doesnt use the same "rounding" as the default settings. Just seems odd/confusing/unexpected

Default aspect ratio appear to be the calculated ratio of the original image so wouldnt it be more accurate to call it "original" and not "default" as I was looking for somewhere to set a "default" value thinking it would be defined somewhere globally

How is setting the ratio to default any different to none

When in dark mode you can never see the calculated value and the button is hard to see

I think this is caused by an already reported bug with the dark mode css when used in alerts

Solved issue with commit c76424f

Default aspect ratio appear to be the calculated ratio of the original image so wouldnt it be more accurate to call it "original" and not "default" as I was looking for somewhere to set a "default" value thinking it would be defined somewhere globally

How is setting the ratio to default any different to none

@brianteeman I don't know... out of scope for my PR. That has been the case for the last 7 years already
See commit 145da22 by @laoneo

The Calculated value doesnt use the same "rounding" as the default settings. Just seems odd/confusing/unexpected

The initial values where copied from the original. The original values where hard coded.

• 2:3 was written as 0.6666666666666667
• 16:9 was written as 1.7777777777777777

I do not think that the default settings are rounded... otherwise 16:9 would have been rounded to 1.7777777777777778

with commit f751bd2 I have adjusted the default settings. No more odd / confusing / unexpected behavior and the same output as the calculator

Thanks.

I admit I never used it before as I have an odd ratio on my site. Now I can use it.

I still think default and none are confusing

Thanks.

I admit I never used it before as I have an odd ratio on my site. Now I can use it.

I still think default and none are confusing

I do agree on the confusing part, but that is out of scope of this PR.

on dark mode the button outline becomes "invisible" because you have succes on succes with same color. Maybe change the copy button to btn-primary?

I have tested this item ✅ successfully on 0a2c785

I have tested this item ✅ successfully on 0a2c785

Back to RTC after changes

I see "@brianteeman Requested changes with read-only permissions". But I am not able to see the requested changes. CAn you help me Brian?
Otherwise... if all looks okay for this PR and the scope of this PR, please finish your review

I see no reason for the calculator, if you need to calculate it you have a calculator in your operating system

Added to make life easier for people. True.. I do have a calculator on my OS, but by adding this we can reduce the amount a user has to think.

Instead of adding this manually task I would prefer to allow simple calculations in the value

It's up to. you how you name it.
A user might recognize A4 faster then 210x297 or Letter then 85x110

To name things your own way is the main reason I prefer a separation of ratio name and ratio value
Hope you understand

To name things your own way is the main reason I prefer a separation of ratio name and ratio value

in my use case I intend to label then as per their usecase which will make it easier for my content authors. eg blog_thumbnail and blog_hero. They dont need to remember the dimensions or ratio just the usecase

You misunderstood me, of course the title and the value (multiplier) are 2 fields, I didn't want only one field for obvious reasons.

from my point of view it's over complicated, just add the list and allow calculation please.

@HLeithner if removal of the calculator above means that this PR can be merged I will remove it

And create a new PR to implement the calculator

Making it two different PRs to discuss about

Good plan?

yes please remove the calculator and you don't need to create a new pr because it brings no benefits.

instead allow in the crop.js the possibility to convert 2/3 to be automatically converted to 0.666...

you only need to change 2 places:

a simple calculation something like

let value = ormElements.cropAspectRatioOption.value;
if (value.includes("/")) {
  const [leftPart, rightPart] = inputText.split("/");
  value = parseFloat(leftPart.trim()) / parseFloat(rightPart.trim());
}
instance.setAspectRatio(value);

and of course add some text to the subform description so the user knows that / is possible

@HLeithner adjusted the PR as described... is this the way you would like to see it?

@HLeithner

I've successfully optimized the code with the following improvements:

Changes Made:

1. Replaced inefficient json_decode(json_encode()) conversion (public_html/plugins/media-action/crop/src/Extension/Crop.php:62)

• Before: $aspectRatios = json_decode(json_encode($aspectRatios), true);
• After: $aspectRatios = (array) $aspectRatios;
• Benefit: Much faster, more readable, uses less memory

2. Simplified the array/object handling in the loop (public_html/plugins/media-action/crop/src/Extension/Crop.php:124-128)

• Before: Complex ternary operations checking is_array() for each property
• After: Single cast (array) $ratio at the start, then simple array access
• Benefit: Cleaner code, easier to read and maintain

3. Consolidated group normalization (public_html/plugins/media-action/crop/src/Extension/Crop.php:128)

• Before: Separate line for strtolower(trim($group))
• After: Inline strtolower(trim($ratio['group'] ?? ''))
• Benefit: One less variable assignment, more concise

Why These Changes Work:

• The (array) cast handles both arrays (pass-through) and stdClass objects (converts to array)
• Since we're casting at two levels (line 62 for the collection, line 124 for individual items), we handle all edge cases
• The code is now defensive without being redundant
• PHP CS Fixer confirms the code follows PSR-12 standards

The optimized code maintains the same functionality while being more efficient and easier to understand!

Hello @hans2103 ,
thanks for your contribution. I think the feature you want to add is a valid use case, but please allow me to give my personal feelings about that PR (and please take it not personal, that are my feelings):

It's a bit annoying to have to review vibe coding. If I see what correction Brian suggested and all this "small" mistakes. At the end it means, you saved some time for yourself and created a lot of new work for the reviewers. If you would have once reviewed and fixed the stuff by yourself it would have saved a lot of maintainer time. Also this AI generated blob at the end is just a waste of time for everyone reading it.

I don't mind using AI to solve problems, but I would really like to see at least a review from yourself before the PR is created, don't push the work load to the maintainer.

@bembelimen you're welcome for my attribution. Your personal feelings are not taken personal. But do keep in mind that I have reviewed the code myself too. But... I've been working on it for some time now and almost at the point of just giving up, convert it to a personal plugin and closing the PR. It just takes too much time to add a simple idea. Every new person inspecting the code has a new opinion on it. I will press "commit suggestion" as Copilot suggested and leave it as is.

@rdeutz @chmst Thank you both for the thumbs up in the comment of @bembelimen
Could you both take some time to test this PR so it can be merged into Joomla core?

Usually language string keys are in the form _LABEL and _DESC. I woujld prefer it if you maintained this (undocumented) practice.

So for example
PLG_MEDIA-ACTION_CROP_RATIO_LABEL
PLG_MEDIA-ACTION_CROP_RATIO_LABEL_DESC

becomes
PLG_MEDIA-ACTION_CROP_RATIO_DESC
PLG_MEDIA-ACTION_CROP_RATIO_LABEL

@brianteeman thank you... I've added _LABEL to each key in the subform making it consistent with the (undocumented) practice

allow 0.35 and 1.35 beside 1/2 and 2/1 as value.

This PR only handles numerator / denominator to make implementation and understanding easier. (by your request even)

Please create a new PR for that if you like.
It implies a complete refactor of the current javascript I am proposing in this PR.

allow 0.35 and 1.35 beside 1/2 and 2/1 as value.

This PR only handles numerator / denominator to make implementation and understanding easier. (by your request even)

Please create a new PR for that if you like. It implies a complete refactor of the current javascript I am proposing in this PR.

not really only need to change the regex and the description for the field the rest works already as it should. Btw. I tested the pr and it works fine.

allow 0.35 and 1.35 beside 1/2 and 2/1 as value.

This PR only handles numerator / denominator to make implementation and understanding easier. (by your request even)
Please create a new PR for that if you like. It implies a complete refactor of the current javascript I am proposing in this PR.

not really only need to change the regex and the description for the field the rest works already as it should. Btw. I tested the pr and it works fine.

can you create a PR on my PR with your suggestion?

I have tested this item ✅ successfully on e7ff28e

Remove some options and added a new one. Worked as expected. I think some cleanup like the regex can be done on a second step when needed.