search

ojsde / shariff

The Shariff plugin adds social media buttons to the website without compromising the privacy of website users, using Shariff (https://github.com/heiseonline/shariff) solution.
GNU General Public License v3.0
9 stars 16 forks source link

Update block template #28

Closed dennmuel closed 9 months ago

dennmuel commented 2 years ago

Some of the modifications I had to make when getting the block to work with OJS 3.3.0.10 and bootstrap3 theme.

I also had to fiddle a bit with the CSS, but that might be specific to our instance so I've kept it out of this PR.

ronste commented 2 years ago

Hi @dennmuel ,

thanks for your contribution to the Shariff plugin. I have a question regarding your changes.

You add the template variable 'enableShariffBlock' which is used in the template file. But you hardcode it as 'true'. What is the purpose of this variable? Do you plan for future extensions?

Also, could you probably provide a link to an installation where I can see the final result of your template modification? I wonder if we should add an additional option to the dropdown. Something like "Sidebar with block design". Users would be able to switch between normal 'Sidebar' without title and your version.

What do you think?

dennmuel commented 2 years ago

Hi @ronste To be honest, I didn't put that much thought into this. I just noticed, that the shariff block did not look coherent to other block plugins in my eyes (no heading, no or less padding). Below are examples for the default theme (I.a) and even more evident with the bootstrap3 theme (I.b).

I.a image I.b image

So I toyed around with the <h2>, <div>s and classes. Examples: Default theme (II.a, probably needs some CSS modification), bootstrap3 without (II.b) and with (II.c) some quick and dirty CSS (not included in this PR).

II.a image II.b image II.c image --> Good enough for my purposes. :D

However, then I noticed that, when you change the shariff config option "position" to "publication page" or "footer", the block is still displayed in the sidebar. image image

This seems to have always been the case, but went unnoticed because the block was completely empty. So I looked at the next best block plugin (the language block in this case) and copied the solution with the template variable. enableShariffBlock only gets set to true, when the position parameter is set to "sidebar" - effectively hiding the block, when it is not desired. I'm open to more elegant solutions.

As for your suggestion of a config parameter to toggle between the hitherto and the suggested layout: Sure, why not. :) Personally, I'd expect all block plugins to behave and look the same. IMHO it should take effort to make them behave differently from everything else - not the other way round. So I thought, I'd share my modifications in case they're of use for someone else.

ronste commented 2 years ago

Hi @dennmuel ,

thanks for the details. I will consider this in more detail when I have time to work on this plugin again. However, currently this doesn't have the highest priority for me. Most likely I will only find time for this when upgrading to OJS 3.4.

ronste commented 9 months ago

I will close this PR and merge it into stable-3_4_0