Skip to content

Continuation of #60: Fix Text Overlay configuration using Dependency Injection#62

Merged
joecorall merged 8 commits into
Islandora:2.xfrom
digitalutsc:text-overlay-fix-continuation
Feb 9, 2026
Merged

Continuation of #60: Fix Text Overlay configuration using Dependency Injection#62
joecorall merged 8 commits into
Islandora:2.xfrom
digitalutsc:text-overlay-fix-continuation

Conversation

@Aryan-R-Patel

Copy link
Copy Markdown
Contributor

Closes Issue #59
Closes and builds on top of PR #60

What does this Pull Request do?

This PR is a continuation of the work started by @DibyaGoswami in PR #60 to resolve Issue #59.

What's new?

Original Logic: Retains Dibya's fix to ensure the "Text Overlay" checkbox correctly affects the Mirador viewer.
Changed Logic: Utilizes Dependency Injection by replacing the \Drupal::service('config.factory') calls in TextOverlay.php and MiradorImageTools.php as requested by @joecorall in the previous PR review.

@Aryan-R-Patel

Copy link
Copy Markdown
Contributor Author

Hi @joecorall,

I am a co-op student at the University of Toronto currently working on this issue. I have opened this PR which includes Dibya's original work from the #60 plus the requested Dependency Injection fixes.

Please let me know if any further changes need to be made to successfully merge this.

Thank you!

@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.

@Aryan-R-Patel

Copy link
Copy Markdown
Contributor Author

They should be fixed now!

@joecorall

Copy link
Copy Markdown
Member

@Aryan-R-Patel there's a few more

@Aryan-R-Patel

Copy link
Copy Markdown
Contributor Author

I have pushed the new changes. Hopefully this covers everything!

Comment thread src/Plugin/IslandoraMiradorPlugin/TextOverlay.php Outdated
@Aryan-R-Patel

Copy link
Copy Markdown
Contributor Author

Hi @joecorall, I have implemented these changes. However, the tests still fail since with the old logic everyone had the plugins always enabled. Would you like me to update the test case?

@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.

The issue is the config form saves mirador_enabled_plugins as key/value. So the suggested code changes below along with will get the tests to pass

diff --git a/tests/src/Kernel/IslandoraMiradorPluginManagerTest.php b/tests/src/Kernel/IslandoraMiradorPluginManagerTest.php
index 0d6d497..46b72bf 100644
--- a/tests/src/Kernel/IslandoraMiradorPluginManagerTest.php
+++ b/tests/src/Kernel/IslandoraMiradorPluginManagerTest.php
@@ -31,6 +31,9 @@ class IslandoraMiradorPluginManagerTest extends KernelTestBase {
    */
   protected function setUp(): void {
     parent::setUp();
+
+    $this->installConfig(['islandora_mirador']);
+
     $this->pluginManager = $this->container->get('plugin.manager.islandora_mirador');
   }

Comment thread src/Plugin/IslandoraMiradorPlugin/MiradorImageTools.php Outdated
Comment thread config/install/islandora_mirador.settings.yml Outdated
Comment thread islandora_mirador.post_update.php Outdated
Comment thread src/Plugin/IslandoraMiradorPlugin/MiradorImageTools.php Outdated
@joecorall
joecorall merged commit b843087 into Islandora:2.x Feb 9, 2026
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