Skip to content

#66 Add openseadragon-bookmark url plugin#77

Open
kayakr wants to merge 9 commits into
Islandora:3.xfrom
catalyst:3.x-bookmark-url
Open

#66 Add openseadragon-bookmark url plugin#77
kayakr wants to merge 9 commits into
Islandora:3.xfrom
catalyst:3.x-bookmark-url

Conversation

@kayakr

@kayakr kayakr commented May 22, 2026

Copy link
Copy Markdown
Contributor

GitHub Issue: #66

  • Other Relevant Links (Google Groups discussion, related pull requests,
    Release pull requests, etc.)

What does this Pull Request do?

Adds local copy of openseadragon-bookmark-url and calls bookmarkUrl(). This allows a URL fragment to track x, y, and zoom so that a tiled image can be presented already zoomed in to a feature of interest, e.g. a face in a group photograph.

How should this be tested?

  1. Enable sub-module, drush en openseadragon_bookmark_url.
  2. Visit page with OpenSeadragon viewer enabled.
  3. After zoom, or pan, zoom and x, y coordinates should be reported in browser address bar.
  4. Paste the URL fragment to return to the same coordinates and zoom

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

Comment on lines +1 to +17
<?php

/**
* @file
* Attach bookmark-url.js.
*/

/**
* Attach openseadragon-bookmark-url.js to all pages.
*
* @todo It would be good to target this better, e.g. load only when OSD is loaded.
*
* {@inheritDoc}
*/
function openseadragon_bookmark_url_page_attachments(array &$page) {
$page['#attached']['library'][] = 'openseadragon_bookmark_url/openseadragon_bookmark_url';
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<?php
/**
* @file
* Attach bookmark-url.js.
*/
/**
* Attach openseadragon-bookmark-url.js to all pages.
*
* @todo It would be good to target this better, e.g. load only when OSD is loaded.
*
* {@inheritDoc}
*/
function openseadragon_bookmark_url_page_attachments(array &$page) {
$page['#attached']['library'][] = 'openseadragon_bookmark_url/openseadragon_bookmark_url';
}

Can we just use libraries-extend on openseadragon/openseadragon in this module's libraries file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT library-extends is only available for themes, not modules https://www.drupal.org/docs/develop/theming-drupal/adding-assets-css-js-to-a-drupal-theme-via-librariesyml#s-libraries-extend. I tried using openseadragon_bookmark_url_library_info_alter() to declare a dependency of openseadragon on openseadragon_bookmark_url but that causes a circular dependency.

For now, I've used openseadragon_bookmark_url_preprocess_field() to check if a field withopenseadragon_formatter is being used.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - I guess modules have to use hook_library_info_alter and can't leverage library-extends.

@kayakr

kayakr commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

@joecorall I've added the ability to specify a media type and (string) field to obtain a bookmark URL from, so that it can be shared into javascript via settings, and thus set the zoom, x&y coordinates for an OSD image. That code has been pushed.

I have other code that attempted to form alter the openseadragon settings form to allow the media type and field to be selected but it's broken (when submitted, the original openseadragon settings form loses its injected variables, e.g. http client. I don't need the UI, so that can be addressed as a distinct issue/PR later.

@@ -0,0 +1,5 @@
services:
openseadragon_bookmark_url.config:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is used? If so can we drop this services file and modules/openseadragon_bookmark_url/src/BookmarkUrlConfig.php

@joecorall joecorall left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this PR and the bookmark functionality works. Very nice!

I think we should consider using hook_library_info_alter to add the bookmark JS to OSD when this module is enabled instead of openseadragon_bookmark_url_preprocess_field

}
};

})($, Drupal, drupalSettings, once);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure that referencing what I assume is supposed to be jQuery as $ here is safe. On the other hand, it doesn't seem to be used, so it probably shouldn't be here in the first place? drupalSettings doesn't appear to be used directly, either.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, yeah, the attempt to use $ globally should cause warnings about unresolvable symbols, given Drupal loads jQuery with .noConflict() by default.

Somewhat related: If the aliasing of jQuery to $ isn't necessary (because jQuery isn't actually being used), then the function wrapping the behavior definition as a whole isn't necessary.

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.

3 participants