Comments (24)
gianantoniopini
commented on September 22, 2024
1
Investigating this issue prompted me to start putting in place some basic unit tests around configuration, i.e. specifying configuration via a parameter to the mailgo function or via window.mailgoConfig . This would be useful to start having some tests coverage around the configuration logic. So I will try to complete these tests in the next few days and submit a PR.
from mailgo.
manzinello
commented on September 22, 2024
Thank you for your report!
I think that the problem may be the line 907 on mailgo.ts
customAction.style.display = customActionText ? "block" : "none";
that not checks the mailgoConfig.
@gianantoniopini, what do you think about? If you can't work on this bug surely I will do it, thank you again for your big work.
from mailgo.
homotechsual
commented on September 22, 2024
Side note but @gianantoniopini I really must thank you for this, it is already proving fantastically useful :-)
from mailgo.
gianantoniopini
commented on September 22, 2024
Thank you, I will check this bug. It must be something I've missed. Strange tough as I even put in place a unit test for this scenario.
@manzinello Thanks for the hint, I will review that line of code.
from mailgo.
homotechsual
commented on September 22, 2024
I did see the unit test for this - I'm not a TS expert but I have enough knowledge to just about understand the code :-)
from mailgo.
gianantoniopini
commented on September 22, 2024
@MikeyMJCO I've just tried it with with test HTML:
<html>
<body>
<div>
<a
href="mailto:[email protected]"
data-custom-action-text="Open Docs"
data-custom-action-url="https://mailgo.dev/docs/"
>[email protected]</a
>
</div>
<script>
window.mailgoConfig = {
actions: {
custom: false,
},
};
</script>
<script src="../dist/mailgo.min.js"></script>
</body>
</html>
and it works as expected: the custom link is not rendered. And if I change custom to true in the window.mailgoConfig then the custom link is rendered as expected.
Could you please also try it with the HTML above ?
from mailgo.
gianantoniopini
commented on September 22, 2024
It seems like it works correctly when using window config, but does not work if the config is passed to the mailgo function.
from mailgo.
manzinello
commented on September 22, 2024
I have to make some checks about this. The difference with in type of action is that it could be defined for a link but not for another (because the attributes are passed as data- in html), this is something that for the other actions is not present. The behaviour of other actions is to call a particular URL with particular parameters and then there is the configuration that give the possibility to show/hide actions, in a generic way. In this case we have a config but maybe an element that says something else. For example: what happens if we set config for custom actions false but we specify the attributes of the custom actions in the link? At the moment I think that the custom action appears. So maybe we have to define a priority for this particular type of action.
I will also make some tests about this.
from mailgo.
homotechsual
commented on September 22, 2024
Yes @gianantoniopini it does seem to work correctly if I replicate your example. Unfortunately we're invoking this by passing the config to the mailgo function.
For this to work we'd either need to be able to pass the custom action in the config or enable/disable in the config and always have the custom action attributes present.
from mailgo.
gianantoniopini
commented on September 22, 2024
I've started putting in place a unit test with the config passed to the mailgo function, setting the "custom" action to false and it actually seems to work as expected also in this scenario: the custom link is not rendered.
I will test this more next week.
from mailgo.
gianantoniopini
commented on September 22, 2024
@manzinello I've checked line 907 in mailgo.ts :
customAction.style.display = customActionText ? "block" : "none";
I believe that it is fine for this line of code to not check the mailgoConfig as that check is already performed in the mailgoPreRender function. The mailgoPreRender function sets the customActionText variable based on the value of the new "custom" config attribute and the value of the data-custom-action-text attribute (this is done in lines 714 -> 728). Unless I'm missing something.
@MikeyMJCO How are you installing mailgo in your project? Do you install it via script (<script src="../dist/mailgo.min.js"></script>) or via npm install mailgo (and then using import mailgo from "mailgo";) ?
Also you could have the option to enable/disable the custom action for a given <a> element by setting its data-custom-action-text and data-custom-action-url attributes to a valid text/url or to empty ("") string if you want to disable the custom action. (Setting the data attributes to an empty string has the same effect as not specifying the data attributes). So you could have the option to always keep the "custom" config attribute to its default value of true and then toggle the visibility of the custom action by setting the data-custom-action-text and data-custom-action-url attributes on each <a> element accordingly.
from mailgo.
homotechsual
commented on September 22, 2024
Hi @gianantoniopini. We're including with <script> . I think that's what we're going to have to do. Check the config first and then unset the data attributes if the custom action is disabled in our settings.
from mailgo.
homotechsual
commented on September 22, 2024
Not sure why the config as above isn't working though, my understanding is that it should be possible to have the data- attributes specified but override.
from mailgo.
gianantoniopini
commented on September 22, 2024
@MikeyMJCO I believe that if you are including mailgo with <script> and you want to specify your own configuration than you need to use window.mailgoConfig. The mailgo(mailgoConfig?: MailgoConfig) function should be used only if you are installing mailgo via npm/yarn. At least this is my understanding, but maybe @manzinello can correct me on this.
from mailgo.
homotechsual
commented on September 22, 2024
The full code we're using is here: https://lab.civicrm.org/extensions/commlink
The mailGo library is in node_modules and we're initialising here: https://lab.civicrm.org/extensions/commlink/-/blob/master/js/commlink_contactsummary.js
So I mispoke earlier, we are using window.mailgoConfig
from mailgo.
homotechsual
commented on September 22, 2024
I think I've spotted the issue :-)
We're calling configure after the functions run!
from mailgo.
gianantoniopini
commented on September 22, 2024
I'm not sure if it is relevant, but check this doc . window.mailgoConfig needs to be set before the mailgo script is added (as in the sample HTML above).
from mailgo.
homotechsual
commented on September 22, 2024
Yeah, now it works fine!
The issue the placement of our mailgo.Configure() call: https://lab.civicrm.org/extensions/commlink/-/blob/master/js/commlink_contactsummary.js#L110
from mailgo.
gianantoniopini
commented on September 22, 2024
Aha! Ok, good to know it is working fine now :-)
from mailgo.
homotechsual
commented on September 22, 2024
I'm going to close this - thanks for the help!
from mailgo.
gianantoniopini
commented on September 22, 2024
You are welcome!
from mailgo.
manzinello
commented on September 22, 2024
I've read everything now, I'm glad that the problem has been solved. Thank you so much!
from mailgo.
manzinello
commented on September 22, 2024
As always, thank you so much @gianantoniopini for your work.
from mailgo.
gianantoniopini
commented on September 22, 2024
@manzinello My pleasure.
from mailgo.
Related Issues (20)
- Does not work with empty recipient field HOT 1
- Custom actions? HOT 7
- Export of validateEmail function HOT 7
- Add unit tests HOT 8
- Run unit tests automatically on each push or pull request HOT 2
- Add unit tests around configuration
- Error when using mailgoDirectRender HOT 2
- Custom Actions with Tel Not Displayed HOT 1
- Set Tel Title HOT 1
- responsive setting HOT 3
- Vue/Buefy mailgo doesn't work in production. HOT 3
- Question: hide body and subject in modal HOT 14
- Doesn't work in links inside dynamically created elements HOT 3
- Tel links with extensions HOT 3
- mailgo modal doesn't open when clicking on telephone like if you exclude a telephone action e.g. whatsapp using window.mailgoConfig HOT 2
- Subject line opens up in 'to' address in gmail / direct render HOT 4
- Clicking the 'copy' button only copies the 'to' address. HOT 6
- Dark theme is not supported with direct render. HOT 8
- Passing a dictionary with configuration options to the direct renderer had no affect when using direct render. HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from mailgo.