-
Notifications
You must be signed in to change notification settings - Fork 24
Use of JS Promise för aply and save in image edit view + some modal messages + a modal progress #561
base: dev
Are you sure you want to change the base?
Conversation
…essages + a modal progress
Thanks for that. Gave it the JS gurus for review. |
media/com_media/js/edit-images.js
Outdated
|
||
// Update the progress bar | ||
Joomla.MediaManager.Edit.updateProgressBar = function(e) { | ||
let value = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please no let here, this is not polyfilled
@dgrammatiko And const for objects is allowed? |
@schnuti sorry I should phrase that better, you need to have ES5 code here... |
I removed the inline CSS style for progress bar.
|
Only possible needed texts
I found out how to add the text strings to JS. Do I have to minify the Javascript somehow? |
@kasvith Nice idea! Could you please open a new issue? This is about the Edit view. I guess something like your idea is possible. |
I will open this, for the edit are i prefer just a message as "Item saved" because a progress bar is not suitable here for me |
@kasvith The idea is that something should happen when you click on the button. The progress bar gives you both started and ended. I'm trying to make it reusable. |
I like the idea of a progress bar, especially as editing now also works with cloud adapters, the upload can take longer. |
ahh yes we now support editing remote files too totally forgot it |
Filename in the message pop up. Progress into 2 separate functions
You can do |
@laoneo Thanks! I have only fragments of a "modern" developer environment. node did it. I have no more open issues. A few comments should be removed before a merge. Ready for a normal test. |
Promises do not work in IE 11 https://caniuse.com/#feat=promises. So either way, we use a polifill, do a fallback or just got with the onSuccess function in Joomla.request. But in the current state it will break IE11. |
Promises should be polyfilled whenever there is a web component in the page (for backend that is always due to alerts) |
So can we load the polyfill in this pr here too? |
it should be loaded already, unless for some reason we're rendering the component and not the index.php.
|
hmm weired, can you see #565 @dgrammatiko |
Actually I figured it out, in J4 I've removed the webcomponents-bundle.js in favour of webcomponents-sd-ce-pf.js, but it seems the second one is missing all the mentioned polyfills on my previous comment. |
@dgrammatiko are you going to fix this in upstream? |
@laoneo I'm waiting for an answer here: #565 (comment) from @kasvith or anyone with IE11 Also @kasvith can you check one more thing: change |
@dgrammatiko there seems to be an issue with upstream, i will check ASAP when it fixed |
@dgrammatiko cant check until #565 passes |
Pull Request for Issue #497 .
Summary of Changes
Testing Instructions
Please first review this code. It works without any known problems but I know some edit has to be done.
So first a few questions.
Edited 1x!