blockonomics / prestashop-plugin

Accept bitcoins on your website, payments go directly into your wallet
Other
16 stars 22 forks source link

Support to Display Enabled Currencies and relevant icons #137

Closed thisisayush closed 2 years ago

thisisayush commented 3 years ago

Fixes #130

image

thisisayush commented 2 years ago

Adds an option in Admin to configure the height of the Logo:

Admin View:

image

Customer View:

On No Value Set i.e. Default image

On Value set to 0 image

On Custom value: 20px image

thisisayush commented 2 years ago

@thisisayush, let me know your thoughts if it is possible to use setAdditionalInformation to also update the logo images, as this may allow us to use btc-icon.png and bch-icon.png, without the need for btc-bch-icon.png. This method will be more scalable in the future, however, might not be required at the moment if it will add too much complexity.

Yes, we can pass the path to image via Smarty Context. Prestashop's Form Helper supports file type as well for configuration. Let me test if we can use that for uploading custom image. Will update you on the same.

thisisayush commented 2 years ago

@thisisayush, let me know your thoughts if it is possible to use setAdditionalInformation to also update the logo images, as this may allow us to use btc-icon.png and bch-icon.png, without the need for btc-bch-icon.png. This method will be more scalable in the future, however, might not be required at the moment if it will add too much complexity.

Yes, we can pass the path to image via Smarty Context. Prestashop's Form Helper supports file type as well for configuration. Let me test if we can use that for uploading custom image. Will update you on the same.

After researching and trying the basics, yes we can provide the option for the user to upload custom logos, but PS will not manage the file itself, take a look at Banner Module (https://github.com/PrestaShop/ps_banner/blob/dev/ps_banner.php#L127), the uploaded file after being validated needs to be moved, so we can move it to module's directory or someplace else, and we can store that path in the configuration.

DarrenWestwood commented 2 years ago

After researching and trying the basics, yes we can provide the option for the user to upload custom logos, but PS will not manage the file itself, take a look at Banner Module (https://github.com/PrestaShop/ps_banner/blob/dev/ps_banner.php#L127), the uploaded file after being validated needs to be moved, so we can move it to module's directory or someplace else, and we can store that path in the configuration.

I do not want the user to upload a logo. I would like to know how much work will be involved in removing btc-bch-icon.png, and instead using btc-icon.png and bch-icon.png? Bitcoin only

<img src="/prestashop/modules/blockonomics/views/img/btc-icon.png">

Bitcoin Cash only

<img src="/prestashop/modules/blockonomics/views/img/bch-icon.png">

Both Bitcoin and Bitcoin Cash

<img src="/prestashop/modules/blockonomics/views/img/btc-icon.png"><img src="/prestashop/modules/blockonomics/views/img/bch-icon.png">
thisisayush commented 2 years ago

Not much, we can pass the array of enabled currencies using additional information and use frontend script to add them in place.

DarrenWestwood commented 2 years ago

Not much, we can pass the array of enabled currencies using additional information and use frontend script to add them in place.

Okay lets use this method instead, and remove btc-bch-icon.png.

https://github.com/blockonomics/prestashop-plugin/pull/137/files#r770593439 also still needs to be addressed.

thisisayush commented 2 years ago

Not much, we can pass the array of enabled currencies using additional information and use frontend script to add them in place.

Okay lets use this method instead, and remove btc-bch-icon.png.

https://github.com/blockonomics/prestashop-plugin/pull/137/files#r770593439 also still needs to be addressed.

To implement this, we'll have to add the helper div back again (https://github.com/blockonomics/prestashop-plugin/pull/137/commits/e66a37d4e283ed6e6adf07c9291ef2b4ca70b30d#diff-3e244bd2586c70ef3e884b2b1a4415d076255a7d46b56bfb5779a611a3743e28R20-R26) to get the target where we want to append the images. I've pushed the new changes with this back to review. It's tested and working. Will apply PS Validator fixes if it looks good.

Frontend Rendering: image

Rendered Script: image

UI: image

DarrenWestwood commented 2 years ago

views/img/btc-bch-icon.png should be removed, and btc-icon.png/bch-icon.png updated to use .svg images.

DarrenWestwood commented 2 years ago

.querySelector("label") is not working correctly with some themes. We may be better off using setLogo, to set the default to btc-icon.png and then replacing this image with the active images

var image = document.querySelector("img[src*='blockonomics/views/img']");
var image_container = document.createElement("span")
{foreach $blockonomicsEnabledLogos as $logo}
    img = document.createElement("img")
    img.src = "{$logo|escape:'htmlall':'UTF-8'}"
    img.style.height = "{$blockonomicsLogoHeight|intval}px"
    img.style.paddingRight = "5px"
    image_container.append(img);
{/foreach}
image.parentNode.replaceChild(image_container, image);

Using setAdditionalInformation is also creating an empty information area when the checkbox is selected. Is there any way to prevent this? Prestashop(1)

thisisayush commented 2 years ago

.querySelector("label") is not working correctly with some themes. We may be better off using setLogo, to set the default to btc-icon.png and then replacing this image with the active images

var image = document.querySelector("img[src*='blockonomics/views/img']");
var image_container = document.createElement("span")
{foreach $blockonomicsEnabledLogos as $logo}
    img = document.createElement("img")
    img.src = "{$logo|escape:'htmlall':'UTF-8'}"
    img.style.height = "{$blockonomicsLogoHeight|intval}px"
    img.style.paddingRight = "5px"
    image_container.append(img);
{/foreach}
image.parentNode.replaceChild(image_container, image);

Using setAdditionalInformation is also creating an empty information area when the checkbox is selected. Is there any way to prevent this? Prestashop(1)

We can hide it using CSS. Let me try something.

thisisayush commented 2 years ago

Updated the commit. Here's how it looks in Dev:

When Payment Mode is selected as Blokconomics: image

image

Also added some properties to the logos parent container to align the logos with some themes.

blockonomics commented 2 years ago

Tested looking good. Can you do the following changes that will update the text


diff --git a/blockonomics.php b/blockonomics.php
index c6a91e6..7830d5b 100755
--- a/blockonomics.php
+++ b/blockonomics.php
@@ -565,10 +565,10 @@ class Blockonomics extends PaymentModule
                 ),
                 array(
                     'type'     => 'text',
-                    'label'    => $this->l('Payment Logo Height (px)'),
+                    'label'    => $this->l('Pay by bitcoin icon size'),
                     'desc'     => $this->l(
-                        'Height of logo in Checkout Page in pixels.
-                         Set to 0 to disable logo.'
+                        'Size in pixels.
+                         Set 0 to disable icon'
                     ),
                     'name'     => 'BLOCKONOMICS_LOGO_HEIGHT',
                     'required' => false,
thisisayush commented 2 years ago

Tested looking good. Can you do the following changes that will update the text


diff --git a/blockonomics.php b/blockonomics.php
index c6a91e6..7830d5b 100755
--- a/blockonomics.php
+++ b/blockonomics.php
@@ -565,10 +565,10 @@ class Blockonomics extends PaymentModule
                 ),
                 array(
                     'type'     => 'text',
-                    'label'    => $this->l('Payment Logo Height (px)'),
+                    'label'    => $this->l('Pay by bitcoin icon size'),
                     'desc'     => $this->l(
-                        'Height of logo in Checkout Page in pixels.
-                         Set to 0 to disable logo.'
+                        'Size in pixels.
+                         Set 0 to disable icon'
                     ),
                     'name'     => 'BLOCKONOMICS_LOGO_HEIGHT',
                     'required' => false,

Done

blockonomics commented 2 years ago

@DarrenWestwood if you approve this, we can merge and do a release