Remove jquery

[Dananji]

Changes in this PR;

• Removed jQuery related dependencies from the Gem and replaced the jquery.treetable JS plugin with Wunderbaum, a modern vanilla JavaScript tree grid component.
  • Removed jQuery-related gem dependencies and related code
  • removed jquery.treetable JS and CSS dependencies including cleanup on vendor/assets
  • re-wrote jQuery code in behavior.js using vanilla JS
• Integrated wunderbaum JS library
  • added bootstrap-icons for iconMap support for file browse interface
  • added a custom adapter to convert server data from browse-everything for wunderbaum
• Styling updates
  • Added Bootstrap 5 style sheet from Avalon's fork to test this in Avalon and kept it in place (this can be removed if needed)
  • Fixed some of the broken CSS in Bootstrap 3 style sheet
  • Added new styling to work with new JS library
• Asset management
  • Replaced files in vendor/assets with files from the new libraries (wunderbaum and bootstrap-icons)
  • Added a postintall script to copy updated asset files from node_modules to vendor/assets for easy management of these assets
  • Updated Rails engine config for new assets (bootstrap-icons fonts)
• Other changes
  • Fixed ActionCable deprecation warning
  • Updated README with migration guide for users upgrading from old jQuery version

NOTE:
While testing this in different browsers I was seeing some twitching with scroll behavior when auto-expanding was used to select nested folders/files. This was happening intermittently for me, i.e. sometimes scrolling works as expected without twitching.
Steps to reproduce;

• Open a file system folder with nested subfolders and files in them
• Click the checkbox next to the root-level folder, which then expands each nested folder recursively and selects the files in the tree-like structure
• Try to scroll to the bottom of the file browse view which sometimes twitches and jumps back up

[jrochkind]

I have time this afternoon and tomorrow to review this!

I will try to test it out in my app, and/or a fresh app of some kind.

Will I find instructions in the README as to how to include the new JS in my app? I'm getting a bit confused by the README, which seems to still mention jquery+treetable installation?

TO BE FAIR -- the README already didn't succesfully include instructions for actually getting started with b-e, and I don't think the generators had been working either with a fresh Rails app! Do you mean to fix that in this PR, should this PR provide a readme/generators that actually work? Or do you mean to leave that unfixed for another day?

Thank you for any additional orientation you can give me to reviewing this PR!

[jrochkind]

Also I think we can defnitely drop Bootstrap 3 support (and even 4 if it's inconvenient?) at this point, for upcoming 2.0 release.

[Dananji]

Thank you taking the time to review this work @jrochkind ! Your feedback on the installation experience is very helpful.

Will I find instructions in the README as to how to include the new JS in my app? I'm getting a bit confused by the README, which seems to still mention jquery+treetable installation?

You are correct. The README in the PR is a bit confusing because it was partially updated. I guess, I wanted to document the steps I followed, which I probably shouldn't have done in the README. I will move these to a different place for the time being.

TO BE FAIR -- the README already didn't succesfully include instructions for actually getting started with b-e, and I don't think the generators had been working either with a fresh Rails app! Do you mean to fix that in this PR, should this PR provide a readme/generators that actually work? Or do you mean to leave that unfixed for another day?

To be completely honest, I have not looked into the state of the Rails generators at all yet, and I'm not familiar with them. So, I might need some time to study this and understand what needs to be done :)

The primary goal of this PR was the substantial technical lift of removing jQuery and replacing jquery.treetable plugin with Wunderbaum. I would prefer to keep the scope focused on the core migration logic for now.

Would it be acceptable to address these issues with the generators and a full overhaul of the installation guide in a separate, follow-up PR?

I think I didn't add any instructions on how to run this because I didn't notice a difference :)
Just to check if it needs any new instructions I ran this as a new installation in my local environment just now. And here are the steps I followed;

1. git clone the repo
2. run bundle install (I set [email protected] using rbenv)
3. then run bundle exec rake db:drop db:create db:migrate
4. ./bin/rails server which started a server at http://localhost:3000/

Hope this helps! Please let me know if you have any questions.

[jrochkind]

Thanks for that context @Dananji -- and for this work!

I think leaving them in the README is fine for now, to keep everything in one place!

Before we have a full 2.0 release, I think we need to attend to the installatino process -- but it doesn't need to be part of this PR!

I'd say leave em in the README for now, thanks! That context was super helpful for me!

[jrochkind]

I'm confused about your last instructions... you're saying you cloned THIS repo, and ran ./bin/rails from THIS repo? Oh wow, I guess this repo does have a bin/rails! Wacky!

The instructions for getting browse-everything installed into your OWN app (not the browse-everything test app!) is the trick -- which is totally not THIS PR's job to solve, cool cool.

[jrochkind]

I had a comcast outage today and probably ran out of time (and I don't work Fridays), but please ping me again if you don't hear back, i want to provide review comments! Warning, i think I personally may need/want some more discussion to understand goals and ideas for future before I personally put stamp of merge approval on it, maybe @cjcolvar can join us too, or we could even have a zoom about it? But first, I'll finish exploring and leave some review comments!

[jrochkind]

Thanks for taking the time to PR this and provide context! Removing JQuery was something on my list too, that I wasn't sure how to approach. (I hadn't managed to find wunderbaum, or the other competitors I've now found searching for 'things like wunderbaum'!)

The following is a "code review" in the sense of a "book review" -- it's my thoughts and an opening of a discussion, not unilateral commands, I am not the boss of b-e!

Would love to have a discussion feedback with @Dananji and @cjcolvar and any other interested parties.

We don't need to solve everything in this PR -- but I think we do need to not make things worse. Not always clear what makes things worse. I am still using b-e off this git/release, and will need to upgrade for supporting new Rails or BL or bugfixes, so what's merged to master even temporarily ideally won't make things a lot harder for me.

Clear on the goals?

My own goals for removing JQuery were primarily a hope of saving bytes in my delivered JS. But this doesn't do that so much, I don't think? All following counts are uncompressed JS.

• JQuery (88 Kb) + treetable (16 Kb) + treetable.css (0.6 KB!) == 105 KB
• wunderbaum.umd.min.js (95kb) + wunderbuam-adapter.js (3kb) + wunderbaum.jss (20kb) == 119 KB

So if I am able to remove jQuery from my project, it's about the same, a bit bigger. If I'm not, I'm actually adding quite a few bytes. [NOTE Code currently doesn't include the wunderbaum.umd.min version counted above, but the full version that is ~300 KB, making it much worse? Should prob include teh .min if we go forward?]

But you may have other reasons than file size to want to eliminate jquery? Let's be clear on them? Maybe this is easier to interact with with JS or CSS to customize? Or the UI is much better? Better browser compat? more accessible? (!?!?) Or other?

Regardless, if Avalon is finding this approach good and is really into keeping it, I would be willing to go along and take the bytes hit for now just to get others helping to maintain this instead of just me! But let's be clear about why we're doing it?

Accessibilty

Sub-question -- do we know if wunderkaum is good/better for accessibility? This thing with lots of dynamic content and hide/show disclosures risks being really not accessible, it would be nice to get accessibility with a big change like this.

There may be options that are more accessible? I found ui-js-tree-wc that is specifically designed for accessibilty and is very small and has the basic features we might need (but no grid view), but isn't very maintained and might need a lot of custom CSS to add grid layout etc.

Icons via alternative to bootstrap-icons?

Bootstrap-icons is default icon provision for wunderbaum -- but support for fonticons default actually adds a lot of complexity to this, a lot of special handling for the font, vendoring an additional dependency.

wunderbaum allows configuring arbitrary other icons

I would strongly recommend we use locally embedded SVG for icons, using data: URIs to actually put the SVG the CSS -- this will remove the need for all that extra handling with bootstrap-icons and make integration a lot easier, including with future improvements. Since bootstrap-icons are MIT licensed, i believe we could even use the very same icons if we like them, but embed them as inline data: in the CSS.

vendoring, can we leave some provenance

Could the new ./script/vendor-assets.sh be modified to leave a README at vendor/assets/ subdirs it adds to, saying that these have been auto-added by that script, and what versions of the dependencies were added? (Removing bootstrap-icons would simplify this too).

asset integration with local app

We've inherited a bad integration story, where bundler is all that's supported, when bundler is no longer supported by recent Rails, and generators probably don't work with recent rails.

It's fine that this PR isn't meant to address that, so just "vendors" all dependencies locally, which was the current status quo.

But we don't want to make it harder to make the integration step, or for people to hack out integrations on their own -- this does a bit, by adding a lot more files dependencies, and by making what was before CSS into SCSS.

I think removing bootstrap-icons dependency will help. If the .scss could be turned into straight .css, that woudl also help -- a lot of people don't want .scss in their pipelines anymore. I'm not sure if there are any specific features we are using .scss for. And avoiding @import, but letting the (for now) sprockets main file do require will actually make it more flexible for integration I think.

We may want better instructions in README -- the README started out pretty confused and not always accurate, and the new grid that's there is great for context (I at first didn't realize I had to scroll right to see it all!), but is more like notes (great notes!) for what we might want ot put in README?

Testing in my app

In my own app, I don't use bundler, so with previous browse-everything I just copied-pasted all the files to be vendored in my own app.

I set out to try a test using that same approach. I got stopped by a couple things.

My app used the browseEverything() JS API to add dynamically, instead of data attributes. New, only data attributes supported. Ran out of time/energy to refactor my app for that -- do we want to still support a JS API initializer as an alternative? Shouldn't be too hard.

Additionally, I might want to actually use wunderbaum via an actual npm dependnecy in my app that does use npm dependencies -- i think this should be possible with the JS as it is, but the CSS not sure, the way it uses local SASS @import. Removing bootstrap-icon should make it easier to figure this out i think.

I prob could have gotten a local demo in my real app working with more time, but ran out out of it for this review.

[Dananji]

@jrochkind Thanks for the feedback! To address your points:

Goal and scope: The primary motivation for this PR for me was to remove jQuery, which aligns with our broader efforts in Avalon to eliminate it as a dependency. While reducing the gem’s byte size is definitely a plus—and fits well with improving asset management. And we can certainly address these broader asset management improvements in a follow-up PR to keep the scope manageable in this work to remove jQuery dependency?

Accessibility (a11y): Wunderbaum is a re-write of FancyTree, which was built with a11y in mind. Wunderbaum carries that forward with solid keyboard navigation and aria-* labels. While there’s always room for more fine-tuning on the a11y front, my comparison shows that this implementation provides a similar level of accessibility support to the previous treetable plugin. As I was playing around with this, I realized there's a couple more changes I need to do get the keyboard a11y enabled in this implementation and I will work on it in this PR.

Icons: I agree that adding bootstrap-icons as a new dependency complicates asset management and increases the footprint. I’ll work on updating this to use locally embedded SVGs via the icon-map option instead.

Testing in app: Would you be able to try the implementation in the dummy-app in this PR?

I'll push those icon and a11y changes soon!