omeka-s-modules / Sharing

Omeka S module for sharing content via 1) Social Media and 2) Embedding Omeka S content in other sites
GNU General Public License v3.0
4 stars 6 forks source link

Feature/blocks #44

Closed Daniel-KM closed 4 months ago

Daniel-KM commented 4 months ago

Simplified code with a view helper and implemented page block, resource block and single button.

jimsafley commented 4 months ago

Thanks for the improvements. We're considering the request.

One thing: the Tumblr button is broken in a "single button." image

Daniel-KM commented 4 months ago

It worked fine in firefox, but google don't want you to use it. So I added a min-width.

jimsafley commented 4 months ago

A couple more issues:

TypeError
count(): Argument #1 ($value) must be of type Countable|array, null given

Details:

TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in /var/www/html/omeka-s/modules/Sharing/Module.php:211
Stack trace:
#0 /var/www/html/omeka-s/modules/Sharing/Module.php(211): count()
#1 /var/www/html/omeka-s/vendor/laminas/laminas-eventmanager/src/EventManager.php(319): Sharing\Module->addSharingMethods()
#2 /var/www/html/omeka-s/vendor/laminas/laminas-eventmanager/src/EventManager.php(171): Laminas\EventManager\EventManager->triggerListeners()
#3 /var/www/html/omeka-s/application/src/View/Helper/Trigger.php(60): Laminas\EventManager\EventManager->triggerEvent()
#4 /var/www/html/omeka-s/vendor/laminas/laminas-view/src/Renderer/PhpRenderer.php(407): Omeka\View\Helper\Trigger->__invoke()
#5 /var/www/html/omeka-s/application/view/omeka/site/item/show.phtml(11): Laminas\View\Renderer\PhpRenderer->__call()
#6 /var/www/html/omeka-s/vendor/laminas/laminas-view/src/Renderer/PhpRenderer.php(519): include('...')
#7 /var/www/html/omeka-s/vendor/laminas/laminas-view/src/View.php(194): Laminas\View\Renderer\PhpRenderer->render()
#8 /var/www/html/omeka-s/vendor/laminas/laminas-view/src/View.php(222): Laminas\View\View->render()
#9 /var/www/html/omeka-s/vendor/laminas/laminas-view/src/View.php(187): Laminas\View\View->renderChildren()
#10 /var/www/html/omeka-s/vendor/laminas/laminas-mvc/src/View/Http/DefaultRenderingStrategy.php(98): Laminas\View\View->render()
#11 /var/www/html/omeka-s/vendor/laminas/laminas-eventmanager/src/EventManager.php(319): Laminas\Mvc\View\Http\DefaultRenderingStrategy->render()
#12 /var/www/html/omeka-s/vendor/laminas/laminas-eventmanager/src/EventManager.php(171): Laminas\EventManager\EventManager->triggerListeners()
#13 /var/www/html/omeka-s/vendor/laminas/laminas-mvc/src/Application.php(360): Laminas\EventManager\EventManager->triggerEvent()
#14 /var/www/html/omeka-s/vendor/laminas/laminas-mvc/src/Application.php(341): Laminas\Mvc\Application->completeRequest()
#15 /var/www/html/omeka-s/index.php(21): Laminas\Mvc\Application->run()
#16 {main}
Daniel-KM commented 4 months ago

The issue is related to the fact that twitter won't display in Chrome if initially hidden and unlike tumblr, it is hard to fix without extra js. So the fix is partial for now.

jimsafley commented 4 months ago

What's the distinction between the placement options "block" and "none"? It seems that there should only be three options:

Where the "Block" option just indicates not to prepend/append to the show pages.

Daniel-KM commented 4 months ago

Yes, "block" and "none (custom theme)" do the same in the code (nothing), but it's more end-user friendly to make a distinction between them. But it's up to you, you can modify anything you want in the module.

Daniel-KM commented 4 months ago

There are some another points to check, the style of the button, or the fact that the embedded icon does not appear in most of the themes.