Added initialization logic when changing container attributes

[devxom]

Added logic to manual update the Likely instance when changing the data attributes of the container on which Likely is initialized

Changes:

• Added the updateOptionFromDataSet method to update the Likely instance configuration after from the data attributes after initialization
• Added an html page for the ability to manually and automatically test data updates when changing data attributes
• Added automatic tests to verify that after changing the data-url attribute on the widget container when opening the model window for sharing, the updated url from the data-url attribute on the container is used
• Added an example to README.md to use the logic for updating the configuration from the date of attributes after initializing the Likely instance

Fixed: #151

Previous version of description

Added logic to initialize the library when changing the data attributes of the container on which Likely is initialized **Changes:** - When initializing a shared sharing widget, the creation of MutationObserver was added to change the attributes of the widget container - Added an html page for the ability to manually and automatically test data updates when changing data attributes - Added automatic tests to verify that after changing the data-url attribute on the widget container when opening the model window for sharing, the updated url from the data-url attribute on the container is used **Important:** to determine that the data-url attribute on the container has changed, it uses the [MutationObserver](https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver) API which works on a slightly narrower number of browsers. **In browsers without support for this API, just this logic will not work.**  Fixed: #151

[vitkarpov]

In browsers without support for this API, just this logic will not work.

Could users initialize the widget once again manually?

[NikolayRys]

Hi, @devxom, thank you for the PR, I appreciate your effort.

However, after some consideration, I've got a feeling that the proposed solution might be unnecessarily complicated. Namely, I think it's sufficient to provide the client with something like likely.reinitialize or likely.updateOptions to use when the attributes are changed.

If the user needs to detect it automatically, they can add a Mutation Observer approach on their own. In the scope of your pull request, I suggest to keep the content of "updateOptionFromDataSet" and remove the observer.

What do you think?

[devxom]

@NikolayRys, i agree. Adding DOM attribute tracking logic is unnecessarily complicated in the library itself.

Within a few days I will update the pull request. Where from the edits will be:

• logic updateOptionFromDataSet
• tests
• an example in README.md that will show how, for example, you can use MutationsObserver from outside the library

[NikolayRys]

Sounds awesome, thank you.

[devxom]

Updated patch and description for pull request. Review please.

[devxom]

@devxom @NikolayRys guys, what do you think about to not introduce a new method but use existing init? It's like there's only one way to (re)initialize the widget.

It sounds logical, I tested it works well. I updated the edits leaving only an example and tests.

[vitkarpov]

@devxom sorry, I'm not sure I get it right. Are you going to change the code in a way it uses init only or leave it as-is? (the latter is fine, just asking)

[devxom]

@vitkarpov I liked the idea of such a simplification, I will update the edits in the near future using init

[vitkarpov]

Gotcha, thanks! 👍

[NikolayRys]

@devxom @vitkarpov Guys, I need your help. I see that we have a test that covers changing attributes with re-initialization after that: https://github.com/NikolayRys/Likely/blob/master/test/index.js#L158

Here's where re-initialization currently happen: https://github.com/NikolayRys/Likely/blob/master/source/index.js#L32

Could you help me figure out, does this pull request add any capabilities beyond that?

[vitkarpov]

@NikolayRys What do you mean? What capabilities exactly?

[NikolayRys]

I'm under impression that we already have a completely functional re-initialization logic in place, could you please confirm/deny this? If it is indeed the case, then do we still need the separate method from this PR that updates attributes from the HTML tag?

[vitkarpov]

Yeah, I see what you mean. It seems that this PR doesn't do anything new 🤔I think we need to reproduce what #151 reports, probably it's already fine.

[devxom]

Really necessary logic to update configuration from date attributes. In this form, Pull Request does not make sense.

@NikolayRys @vitkarpov How do you look at simply describing the approach to do when using dynamic date attributes and adding a test that checks and shows how this happens?

[vitkarpov]

Sounds good to me! 👍 -- Cheers, Viktor

[NikolayRys]

We definitely lacked this information about that in the readme, so at least this part of work in this PR can be kept.

@devxom But about the test - doesn't this test: https://github.com/NikolayRys/Likely/blob/master/test/index.js#L158 already cover it?

[devxom]

@NikolayRys

@devxom But about the test - doesn't this test: https://github.com/NikolayRys/Likely/blob/master/test/index.js#L158 already cover it?

You are right, indeed this test covers this case

This weekend I will update the Pull Request. I will remove unnecessary edits and update changes with edits in README.md with an explanation of how to reinitialize Likely when using dynamic date attributes

[NikolayRys]

@devxom No update yet?

[devxom]

@NikolayRys Reassembled edits based on comments. Please take another look at the code.

[NikolayRys]

Hi, @devxom, thanks for taking care of this.