Improve generator installation experience

[mamrey]

Assess what improvements we can make to the asset generator installation experience. Is it feasible/worthwhile to implement a generator or generators that can install only the specific views desired by the developer user? Is the more appropriate path to have the installer continue to install all views, while better integrating the current rails asset pipeline, and providing excellent documentation to support blacklight users who want more control over what parts of them they want?

The first part of this issue should be to assess the questions above and provide a recommendation to discus and move forward. Afterwards, implement the proposed solution to improving the generator install experience. In any case, we need to have an installer process that can install all the views, and leans in to using the current rails asset pipeline (import maps + propshaft) to minimize any manual setup.

Tasks:

• assess feasible options for improving the generator installer and make recommendation
• implement recommended improvements to the generator
• ensure generator can install all the views for an "easy-mode" experience
• ensure generator considers current rails asset pipeline (import maps and propshaft) and minimizes any manual set up
• add clear and thorough usage documentation

[mksndz]

After looking in to this, it would be possible to control which views get configured, and which required assets are included, via parameters passed to the :install generator. Most of the complexity in the generator supports the slideshow view. The other views have less complexity.

I think we should leave the generators as-is and add an additional section to the README that details options for a more piecemeal installation. This would be helpful for implementers just interested in using the gallery without taking on additional dependencies.

[mksndz]

We could make a small addition to the generator for the 4,x release that checks if Importmaps is defined, and configures the importmaps config and application.js file accordingly. What do you think @mamrey ?

[mksndz]

Related, I added to the Wiki a page that details a more manual installation method. We should augment it with any new asset-related once a 5.x release is available.

https://github.com/projectblacklight/blacklight-gallery/wiki/Manual-Installation

[cgalarza]

We could make a small addition to the generator for the 4,x release that checks if Importmaps is defined, and configures the importmaps config and application.js file accordingly. What do you think @mamrey ?

I am thinking the same thing might be necessary to support esbuild, but I am still trying to understand how esbuild works. If we were to do this for the next 4.x branch, it could reduce the amount of documentation we have to add?

[mksndz]

Core BL has similar logic to handle asset-specific logic in the generator.

Here's what I'd add to the install_generator here.

# install_generator.rb

#...

def assets
  copy_file "blacklight_gallery.css.scss", "app/assets/stylesheets/blacklight_gallery.css.scss"

  if defined?(Importmap)
    append_to_file 'config/importmap.rb', 'pin "jquery", to: "https://code.jquery.com/jquery-3.7.1.min.js"'

    insert_into_file 'app/assets/javascripts/application.js', after: 'import Blacklight from "blacklight"' do
      "\nimport 'jquery'"\
      "\nimport 'blacklight-gallery'"
    end
  elsif defined?(Sprockets)
    append_to_file 'app/assets/config/manifest.js', "\n//= link blacklight_gallery/manifest.js\n"

    insert_into_file "app/assets/javascripts/application.js", after: '//= require blacklight/blacklight' do
      "\n//= require blacklight_gallery/blacklight-gallery"
     end
  end
end

Hopefully once the SASS is converted we can address the style generation issues with the generator when Sprockets isn't around.

[mamrey]

@mksndz and @cgalarza Thanks for looking into this! We do need to consider Importmap in the generator in order to properly install the masonry and slideshow views in a way that 'just works'. The feature tests are written in a way that they still pass without the proper ImportMap configuration. So that's why we have passing tests, but when we look at the internal test app, the views are broken and require the manual intervention described in #192. If we want the generator to 'just work' with Importmap on 4.X, we would need to move forward with @mksndz proposal above.

Unfortunately, this is complicated by the import statement in the masonry and slideshow javascript modules. This imports the Blacklight object from 'blacklight-frontend' instead of 'blacklight'. This change to unify the Blacklight import is available on blacklight 9.X, and there is a draft PR to backport it to Blacklight 8.X, and specifically mentions spotlight. I suspect this may also be related to the issues with esbuild.

So, in addition to moving forward with the small update to the 4.X generator to consider Importmap, we also need to address the issue with the Blacklight import in those modules. I will ticket that work.

Let me know your thoughts on this.

[cgalarza]

@mamrey I am still trying to wrap my head around all of this. Are you suggesting that in 4.X we revert back to importing from blacklight instead of blacklight-frontend? or that we hold off fixing this until 5.X?

[mamrey]

@cgalarza Yes, my initial suggestion was to revert back to using the pre-blacklight 9 way of importing the Blacklight module: import Blacklight from 'blacklight'. This is what the blacklight generator gives to us in our application.js file when using BL 8.

We also had the idea of moving the Blacklight.onload calls out of the masonry and slideshow modules and into the application.js. If this works, then we would always rely on the import that the blacklight generator provides. However, you raised important concerns about how this may affect the npm package, which I believe is bundled with rollup?