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