internetarchive / openlibrary

One webpage for every book ever published!
https://openlibrary.org
GNU Affero General Public License v3.0
5.22k stars 1.37k forks source link

Migrate Colorbox code to jQuery UI dialog #4595

Open jdlrobson opened 3 years ago

jdlrobson commented 3 years ago

Per #1732 we would like to update from jQuery 1 to jQuery 3. One of the big blockers to doing this is our use of the jQuery colorbox library.

The library is used in the following templates: openlibrary/templates/lists/widget.html openlibrary/templates/books/edit/addfield.html openlibrary/plugins/openlibrary/js/add_new_field.js openlibrary/plugins/openlibrary/js/ol.js openlibrary/plugins/openlibrary/js/utils.js openlibrary/plugins/openlibrary/js/patron-metadata/index.js

jquery colorbox makes use of the href option to construct a dialog. For example if href is '#addList' the contents of $('#addList').html() are shown in a dialog.

I propose we replace this library with jQuery UI's dialog

$( btn ).on('click', () => {
  $('#addList').dialog({ dialogClass: 'lightbox' });
});

Api options here: https://api.jqueryui.com/dialog/

Evidence / Screenshot (if possible)

Going to https://openlibrary.org/works/OL47713W/The_Life_and_Death_of_Jason and calling:

 $.colorbox({
             inline: true,
             opacity: "0.5",
             href: "#addList",
             onOpen: function() {
                $('#addList').find("#new-list").ol_validate({
                    // list widget index of the "active"
                    submitHandler: function(form) {
                        Lists.widgets[window.activeListWidgetIndex].create_new_list_submit_handler(form);
                    }
                });
            }
           });

results in:

Screen Shot 2021-02-13 at 8 31 31 AM

Acceptance criteria

QA

Test the following scenarios:

Stakeholders

@jdlrobson

dwaipayan05 commented 3 years ago

The line \$.fn.colorbox.resize(); in addfield.html is dead code and thus removed. Can this be done and sent through a standalone PR ?

jdlrobson commented 3 years ago

The line \$.fn.colorbox.resize(); in addfield.html is dead code and thus removed. Can this be done and sent through a standalone PR ?

It could, but to be honest I don't think it would be a valuable use of time (for you or the reviewer) unless you are planning to take on the entirety of the issue here.

navneetsaluja commented 3 years ago

Might as well learn something new. Working on this. @/jdlrobson Let me know if you have more resources/tips other than the ones mentioned above. Thanks for writing the steps to reproduce everything ☺️

jdlrobson commented 3 years ago

Neat! I'm happy to help out with any challenges you hit here. Definitely will be a good learning opportunity :)

https://www.jacklmoore.com/colorbox/ does a good job of documenting the library we're currently using and https://api.jqueryui.com/dialog/ is what we can hopefully migrate to. I'd suggest playing around with both the libraries and understanding how they work as a first step and how we use colorbox (and if said things are also possible with the dialog).

github-actions[bot] commented 9 months ago

Assignees removed automatically after 14 days.