Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrote uploader without flow.js and prototype.js #91

Merged
merged 8 commits into from
Jan 7, 2025

Conversation

justinbeaty
Copy link
Contributor

As mentioned in OpenMage/magento-lts#2433 (comment), the usage of flow.js is rather pointless. It was probably just a quick solution to replace the old Flash uploader.

Commits

Misc library updates

  • Rewrote some of the toggleVis functions to account for both using the no-display class and style="display: none". Also added a checkVisibility function since some places were using the Element.visible() method from Prototype.
  • Fixed the adviceContainer validation option and allow both the old typo'd name as well as the corrected name.
  • Created an xssFilter function that was used in the uploader code, except rewritten in vanilla js.

Rewrite uploader without flow.js and prototypejs

  • Rewrote public/js/mage/adminhtml/uploader/instance.js without flow.js
  • Cleaned up some of the config options since many aren't relevant any more.
  • Removed IE9 compatibility code that was still somehow in the repo.

Rewrite Downloadable Information Product Tab

  • The Manage Products > Downloadable Information tab needed some updates to work with the new uploader, but this code was so messy that I also rewrote it without PrototypeJS. The code was so old it checked to see if console.log was even a function.
  • There were a lot of small bugs too that have been fixed. Also fixed small things like not being able to disable a link's sample file once you've created it.
  • This part probably took longer than rewriting the uploader itself. I could make a separate PR if wanted, but I figured it's easier to test this all at once.

Other updates

  • Fixes the other pages that use the uploader.

Backwards Compatibility

There are some differences between the old uploader and new one:

The main difference is the events since the old code used Prototype's fire() method. Now that it is using native events, you must use addEventListener either on document.body or on the uploader object itself (which extends the EventTarget class). Also instead of event.memo which is a prototype thing, you'll use event.detail which is native. It also passes an actual object instead of a JSON string for the fileSuccess event. There are also a lot of new events that weren't exposed in the old code. So the whole thing is more flexible.

Another change is that replaceBrowseWithRemove is integrated into the uploader class. Before setting that option would make the uploader fire an "upload:simulateDelete" and "uploader:simulateNewUpload" event, which was then handled by the downloadable information tab (the only place that used this option). It really didn't make any sense, and was probably just a legacy thing from the old Flash uploader.

Another legacy thing was that the uploader endpoints returned the cookie value in the response JSON. I assume the Flash uploader needed that information for making requests. Really not a great thing for those who choose to have HTTP-only cookies.

Testing

  • Manage Products > Images tab
  • Manage Products > Downloadable Information tab
  • CMS > Pages > Content > Insert Image

I think those are the only placed it's used.

@fballiano
Copy link
Contributor

I was keeping an eye on flow.js and this PR is amazing, I couldn't find any fault

@fballiano fballiano merged commit 7f0fe00 into MahoCommerce:main Jan 7, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants