Skip to content

Conversation

@hans2103
Copy link
Contributor

@hans2103 hans2103 commented Nov 7, 2025

Pull Request for no issue

Summary of Changes

This PR will add a subform to plugin media-action/crop to give you the option to configure your own aspect-ratios.
With this PR merged I am able to create a own set of aspect ratios.

Testing Instructions

  • Go To System > Plugins > Media-Action = Crop and notice the lack of options

  • Go To Content > Media ... select the three dots of an image and press "Edit". Open the dropdown at Aspect-Ratio and notice the presence of a pre-defined set of aspect-ratios

  • Apply the PR

  • npm run build:js to compile newly added js

  • Go To System > Plugins > Media-Action = Crop and notice the availability of an aspect-ratio-calculator and a subform to add / remove / edit aspect-ratio options. Add, remove and edit aspect ratios.

  • Save the changes

  • Go To Content > Media ... select the three dots of an image and press "Edit". Open the dropdown at Aspect-Ratio and notice the changed set of aspect ratios

Actual result BEFORE applying this Pull Request

Screenshot 2025-11-07 at 11 37 04 Screenshot 2025-11-07 at 11 37 31

Expected result AFTER applying this Pull Request

Screenshot 2025-11-07 at 11 36 40 Screenshot 2025-11-07 at 11 37 47

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-6.1-dev labels Nov 7, 2025
@brianteeman
Copy link
Contributor

Thank you for fixing those errors

@RickR2H
Copy link
Member

RickR2H commented Nov 19, 2025

I have tested this item ✅ successfully on d79e1f7

Test by code review


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46421.

@exlemor
Copy link

exlemor commented Nov 19, 2025

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 ;(

@richard67
Copy link
Member

@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

@brianteeman
Copy link
Contributor

The system tests are correct!

@brianteeman
Copy link
Contributor

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

@TLWebdesign
Copy link

TLWebdesign commented Nov 19, 2025

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.

Scherm­afbeelding 2025-11-19 om 21 13 10

@hans2103
Copy link
Contributor Author

@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.

@brianteeman
Copy link
Contributor

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

@brianteeman
Copy link
Contributor

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

image image

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

@brianteeman
Copy link
Contributor

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

image

@brianteeman
Copy link
Contributor

image

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

@hans2103
Copy link
Contributor Author

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

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

Solved issue with commit c76424f

@hans2103
Copy link
Contributor Author

hans2103 commented Nov 20, 2025

image 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

@hans2103
Copy link
Contributor Author

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

image

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

@brianteeman
Copy link
Contributor

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

@hans2103
Copy link
Contributor Author

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.

@TLWebdesign
Copy link

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?

for better readability on dark mode
@TLWebdesign
Copy link

I have tested this item ✅ successfully on 0a2c785


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46421.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-6.1-dev RTC This Pull Request is Ready To Commit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants