Are the eslint ignore lines still needed?

eslint-disable-next-line class-methods-use-this

The progress bar in the PR does not show the actual progress.
You also have to implement updateProgressBar(), otherwise it does not make sense.

Are the eslint ignore lines still needed?

eslint-disable-next-line class-methods-use-this

With the current implementation, the build fails when not including them as I'm not using this in the removeProgressBar

The progress bar in the PR does not show the actual progress. You also have to implement updateProgressBar(), otherwise it does not make sense.

This progess bar works just like the progress bar in the media manager, they both use media-loader class which doesn't show the actual progress.
However, I just made another implementation that adds a progress div that's hidden to the layout template

<div id="progress" class="progress visually-hidden">
    <div id="progress-bar" class="progress-bar bg-success" role="progressbar" aria-valuenow="0" aria-valuemin="0" aria-valuemax="100"></div>
</div>

and just toggle it visually when adding / deleting the progress bar

// eslint-disable-next-line class-methods-use-this
createProgressBar() {
  const progress = document.getElementById('progress');
  progress.classList.remove('visually-hidden');
}


// eslint-disable-next-line class-methods-use-this
updateProgressBar(position) {
  const progressBar = document.querySelector('.progress-bar');
  if (progressBar) {
    progressBar.style.width = `${position}%`;
    progressBar.setAttribute('aria-valuenow', position);
  }
}

// eslint-disable-next-line class-methods-use-this
removeProgressBar() {
  const progress = document.getElementById('progress');
  const progressBar = document.querySelector('.progress-bar');
  if (progress) {
    progress.classList.add('visually-hidden');
    progressBar.style.width = '0';
    progressBar.setAttribute('aria-valuenow', '0');
  }
}

This implementation actually shows the progress and utilizes the updateProgressBar function, this is how it looks

I'm not sure if this is the best way to go about solving this, but I would love some feedback, thanks!

@mahmoudmagdy1-1 It seems you have changed the mode (Unix permissions) of file "build/media_source/com_media/js/edit-images.es6.js" from 644 to 755. Could you revert that?

This progess bar works just like the progress bar in the media manager, they both use media-loader class which doesn't show the actual progress.

I know, but that also was wrong, it does not give to User any information about how long it takes.
The animation take 1 sec, but the data processing could take a teens of seconds, or even minutes.
We should avoid such progress representation.

The upload progress will be fixed in #44848

However, I just made another implementation that adds a progress div that's hidden to the layout template

Yes, that probably will be much more useful.

Are the eslint ignore lines still needed?
eslint-disable-next-line class-methods-use-this

With the current implementation, the build fails when not including them as I'm not using this in the removeProgressBar

You could change your code a bit and remove the eslint disable rule, ie:

createProgressBar() {
  this.progress = document.getElementById('progress');
  this.progressBar = document.querySelector('.progress-bar');
  this.progress.classList.remove('visually-hidden');
}

updateProgressBar(position) {
  if (this.progressBar) {
    this.progressBar.style.width = `${position}%`;
    this.progressBar.setAttribute('aria-valuenow', position);
  }
}


removeProgressBar() {
  if (this.progress && this.progressBar) {
    this.progress.classList.add('visually-hidden');
    this.progressBar.style.width = '0';
    this.progressBar.setAttribute('aria-valuenow', '0');
  }
}

Yes, that probably will be much more useful.

You could change your code a bit and remove the eslint disable rule, ie:

Thanks for the feedback, I'll make a commit now with these changes

Please rebase to the v5.3 branch. Thanks.

Thanks for the PR, although it looks like a nice feature, we're currently in a feature freeze state for Joomla! 5.3, so I've rebased it to 5.4 so it can be tested there.

shouldnt that be 6.0? When @HLeithner did the big rebase the other week all unmerged features were moved to 6

shouldnt that be 6.0? When @HLeithner did the big rebase the other week all unmerged features were moved to 6

@brianteeman For now 5.4-dev is ok. We (maintainers and/or RMs) will decide soon if 5.4 or 6.0. If we decide for 6.0 it is easier to rebase again from 5.4-dev to 6.0-dev than it would be the other way around.

@richard67 very confusing for contributors

@richard67 very confusing for contributors

@brianteeman I know. But that should not last long, I hope we will have it clarified soon in general for feature PRs.

especially as so many PR were already force moved to 6

I have tested this item 🔴 unsuccessfully on 8340aad

Tested with current 5.4-dev, before this PR there is no progress bar. After applying this PR and running npm ci there is a big green progress bar on saving the image. There is no progress bar on image edits, like loading for crop, rotate or resize -> therefore this test fails.

It may be helpful to create test images, e.g. with:

• 0,5 MB 400 x 400 px

  • convert -size 400x400 xc: +noise Random -depth 8 -gravity Center -pointsize 60 -fill black -stroke black -strokewidth 1 -annotate 0 "Test Image\n400x400" test-image-400.png

• 50 MB 4.000 x 4.000 px

  • convert -size 4000x4000 xc: +noise Random -depth 8 -gravity Center -pointsize 600 -fill darkred -stroke black -strokewidth 4 -annotate 0 "Test Image\n4000x4000" test-image-4000.png

• You may have to configure:

  • Joomla Media Maximum Size: 100 MB
  • PHP memory_limit = 1024M
  • PHP post_max_size = 100M
  • PHP upload_max_filesize = 100M

Additional the file upload progress bar is a small blue one and the image saving bar is a big green one. Should this not be visual consistency?

Screen shots and a video is uploaded to GitHub afterwards.

https://github.com/user-attachments/assets/d53d9316-f461-4f19-bbac-1edc436a84b9 shows

• Opening a 50 MB image is missing progress bar
• Rotate this image is missing progress bar
• Resize joomla_black.png is missing progress bar

File/Image upload bar is small blue one:

Image saving bar is big green one:

Thank you @mahmoudmagdy1-1 so much for your work on this.
It was the main focus of the maintainers' meeting last night, and @muhme did an amazing job of going through it in detail.

Several things came out, one is that there is an inconsistency with the use, so for example you are proposing it for save but rotation and other editing effects also take time, and with @muhme demo they were sometimes in the order of 5x longer than the save, so it would be ideal to be consistent by adding the same progression to those events as well.

There was a discussion as to whether it should be the Joomla spinner that is shown to allow the user to know that something is happening. But the progress bar was agreed although the large green thick version did seem perhaps too bulky and different from progress we have in other places, the thin blue line.

A good point was raised about why implement it as you have, why not use the html element
https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/progress

This is widely supported now and would be the ideal way to semantically add it to a page.

If this were rebased to 6.0 (5.4 comes out the same time but is not designed for new features but a bridge between majors) then @Bodge-IT and I as the release managers for 6.0 would welcome it as a new feature and help as much as we can to get it accepted.

Look forward to your feedback

Thanks @softforge and after the maintainers advice to test it with throttled network connection, a new feature request was created, please see #45377. The slowest part is opening the image for editing.

Thank you for testing the pr and your input, It's much appreciated.

There is no progress bar on image edits, like loading for crop, rotate or resize -> therefore this test fails.

We can't track real progress for "loading for crop, rotate or resize" because it happens synchronously, and there is no indication sent about the progress happening, but when saving we get xhr.upload.onprogress which sends the real progress that we can use to update the progress bar with real progress,
we can implement a progress bar for loading, that doesn't show the actual progress like the one in the media manager, Just to let users know It's loading.

the large green thick version did seem perhaps too bulky and different from progress we have in other places, the thin blue line.

the progress bar in the other places doesn't track the actual progress as Fedik mentioned, he also said that should be fixed with #44848.

why not use the html element

Sure I'm going to include this in my next commit, I'm just not too sure about how should I deal with the inconsistency of loading the image and the other editing effects.

@mahmoudmagdy1-1 Based on the discussion in the maintainers team I have allowed myself to rebase your PR to 6.0. Let me know if that's not ok for you and I shall change it back.

Let me know if that's not ok for you and I shall change it back.

It's fine for me

We really appreciate the effort @mahmoudmagdy1-1, but it has been decided not to move forward with this PR at the moment, as it introduces some visual inconsistencies. After #44848, it could be worth to work again on the image editing progress bar. Looking forward to your contributions in the future!