[#34885] - [4.0] Redo the image editing api
- Fixed in Code Base
- 27 Jul 2021
- Medium
- Build: 4.0-dev
- # 34885
- Diff
- dgrammatiko:4.0-dev-redo-image-editing-API
User tests: Successful: Unsuccessful:
Pull Request is the second part for the Tabs reworked code: #34813
Summary of Changes
Some changes to the API:
- Now it's async (used to be synchronous, but that won't work for many cases, current libraries, etc)
- Fixing all the problems with:
-
- Empty fields on load
-
- Save needed a click away to update the image
-
- other inconsistencies
Testing Instructions
This PR is not testable with the downladable version
Apply this PR!!!
You need to run npm install
Try to edit some images and using all the existing options
Actual result BEFORE applying this Pull Request
Many bugs
Expected result AFTER applying this Pull Request
No (obvious) bugs
Documentation Changes Required
The old registration was something like:
// Register the Events
Joomla.MediaManager.Edit.crop = {
Activate(mediaData) {
// Initialize
initCrop(mediaData);
},
Deactivate() {
if (!Joomla.MediaManager.Edit.crop.cropper) {
return;
}
// Destroy the instance
Joomla.MediaManager.Edit.crop.cropper.destroy();
},
};Which is changed to:
// Register the Events
window.addEventListener('media-manager-edit-init', () => {
Joomla.MediaManager.Edit.plugins.crop = {
Activate(image) {
return new Promise((resolve /* , reject */) => {
init(image);
Joomla.MediaManager.Edit.plugins.crop.cropper
.setAspectRatio(formElements.cropAspectRatioOption.value);
resolve();
});
},
Deactivate(image) {
return new Promise((resolve /* , reject */) => {
if (image.cropper) {
image.cropper.destroy();
delete Joomla.MediaManager.Edit.plugins.crop.cropper;
}
resolve();
});
},
};
}, { once: true });So most of the changes are just changes in the initialization step:
- there's an event for the registration, on the window, named 'media-manager-edit-init'
- The plugin name is now one level deeper, from
Joomla.MediaManager.Edit.croptoJoomla.MediaManager.Edit.plugins.crop - Both the
ActivateandDeactivatefunctions should return a promise.
@wilsonge sorry...
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_media JavaScript Repository NPM Change |
| Labels |
Added:
NPM Resource Changed
?
|
||
Edit an image:
Uncaught TypeError: this.Undo is undefined edit-images.js:99:5
Click Close button:
Uncaught TypeError: newForm.task is undefined core.js:389:7
Uncaught TypeError: this.Undo is undefined edit-images.js:99:5
Fixing the lint issues without checking the code is not a great idea, should be ok now
I have tested this item
It all seems to work. I did notice some anomalies:
The aspect ratio box default was 'None'. The Edit screen does not have a Help button (it could use the media screen Help). Changing X-axis and Width did not show in the grid. The image is barely visible in 768x1024 screen mode and the grid shrinks strangely. Cannot edit (etc) from list view. In edit mode repeated clicking of Save moves the images left a little with each click (but the saved image is OK).
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/34885.
The aspect ratio box default was 'None'.
Oops I change that in the other PR, will undo the changes here
The Edit screen does not have a Help button (it could use the media screen Help).
This is out of the scope of this PR but should be easy to add a help button here:
Changing X-axis and Width did not show in the grid.
The image is barely visible in 768x1024 screen mode and the grid shrinks strangely.
In edit mode repeated clicking of Save moves the images left a little with each click (but the saved image is OK).
I guess this is for crop, investigating all three issues
Cannot edit (etc) from list view.
This is something for the Media manager
For crop, the image expands height/width accordingly, but not under the other actions. Is this intentional?
For crop, the image expands height/width accordingly, but not under the other actions. Is this intentional?
The image has a max-width: 100% so it should adapt to the parent's size. I didn't touch that part (or the logic), just updated the initialization logic and fixed some obvious mistakes (re attaching listeners, etc). Anyways I will check this again
For crop, the image expands height/width accordingly, but not under the other actions. Is this intentional?
Should be fine now
It is no longer side by side. The settings are at the top and image at the bottom.
It is no longer side by side. The settings are at the top and image at the bottom.
Oops, sorry, handling too many PRs is not easy
The image still expands as seen between crop and resize.
Image size corrected
@dgrammatiko It needs to solve conflicts and update the branch here after recent merge into 4.0-dev.
@dgrammatiko It needs to solve conflicts and update the branch here after recent merge into 4.0-dev.
Yup, and now I hope tests are green
@dgrammatiko You might have to update your branch again because just right now George merged the changed tests into 4.0-dev.
@dgrammatiko System tests failed so it seems you have updated your branch just 2 minutes too early.
All CI checks green ... there must be something wrong ;-)
I don't think the tests cover much of the image editing :P Mainly the list view :D
@dgrammatiko Javascript CS: https://ci.joomla.org/joomla/joomla-cms/46162/1/21
This should be ready for testing now
I don't think the tests cover much of the image editing :P Mainly the list view :D
The best tests, navigate to the page read the title. Test is passing. Yeah...
The best tests, navigate to the page read the title. Test is passing. Yeah...
Lots of the tests still fail on bad changes though :P What does that say :D
What does that say :D
Unstable server?
So the crop filters don't apply - but also don't error if you pick e.g. a portrait crop for a landscape image (so just a general lack of feedback - not sure whether that was the case before or not - hard to tell now the other tabs PR is merged as there's no image preview in 4.0-dev right now.
I also got a 403 back when trying to save it. It's a put request to http://localhost/~george/joomla-cms/administrator/index.php?option=com_media&format=json&mediatypes=0,1,2,3&task=api.files&path=local-images:/banner-og-github.png - will test properly later just in case it's something locally. But somehow doubt it as it's just a simple file upload then edit.
Open an image.
Click Resize tab.
Click Save button.
Image saved.
Click Save button.
Error in console:
Illegal mime type detected: application/octet-stream
Hit the 90 degree rotate button twice. You get kinda weird behaviour.
Hit the 90 degree rotate button twice. You get kinda weird behaviour.
Fixed
If you manually enter text for the crop it seems to have no affect. If you adjust the crop box on the image it works fine.
The clicking of 90 degree's rotation is better but not fixed too.
The clicking of 90 degree's rotation is better but not fixed too.
What's the problem now? The image should cover all the right area (assuming it's big enough)
If you manually enter text for the crop it seems to have no effect. If you adjust the crop box on the image it works fine.
@Quy also mentioned this but the initialization code for cropper is the same AFAICT
What's the problem now? The image should cover all the right area (assuming it's big enough)
Same issue. I just have to click 90 degree rotation 4 times instead of twice :) I'm not speed clicking either to try and break things. Sometimes if you wait long enough the screen area resizes itself back to the correct size.
I also think the aspect ratio dropdown is broken? Not sure if that's related to the cropper code though
Same issue. I just have to click 90 degree rotation 4 times instead of twice :) I'm not speed clicking either to try and break things. Sometimes if you wait long enough the screen area resizes itself back to the correct size.
Ok, so this was a 2 part solution:
- there was some weird css on the parent element of the image
- browser needed some time to rethink the layout/painting
Give it some hard reloading before testing
OK So cropper works great now! Way better. But now from that commit rotation doesn't work. It never leaves the cropper view
It's all broken
I mean rotate may well be acting the same as before. But not sure this is intended :D However it's way better than it has been all evening.
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-07-27 21:37:05 |
| Closed_By | ⇒ | wilsonge |
Thanks!
Release the Kraken
@dgrammatiko There is still much to be fixed.
@richard67 I know, but a new RC is always a good thing...
Sorry guys, but since this pr the reset button is not working anymore.
Has it ever worked? ;-)
Sorry, was joke. Could you open an issue?
This PR should be fixing:
#33843
#31859
#33192