Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve interpretation of attributes #172

Closed
NikolayRys opened this issue Apr 19, 2020 · 3 comments
Closed

Improve interpretation of attributes #172

NikolayRys opened this issue Apr 19, 2020 · 3 comments
Assignees
Labels
3.0 Roadmap for the next version

Comments

@NikolayRys
Copy link
Owner

NikolayRys commented Apr 19, 2020

Status quo

All the params from each widget are being appended to the URL that is being used by the button for the sharing call:
https://github.com/NikolayRys/Likely/blob/master/source/button.js#L216
As I understand, this has been done to unify the way the URL build between all the services.

The problem

This approach doesn't make much sense. Each service constructs the URL in each own way, knows a finite list of parameters, or doesn't use different parameters at all.

  1. This leads to the creation of nonsensical URLs.
  2. It prevents using custom parameters as to "disable counter" or "default message" for each of the services since they will mess with URL creation.
  3. It makes adding new services more difficult.

Proposed solution

Initial change for 2.5:

Add a concept of "known params" for each service. For example, "data-via", but not "data-hello" to Twitter. Continue attaching all the parameters to the URL, but throw console warning each time the parameter not recognised.

Breaking change for 3.0:

Remove the shared logic of constructing URL out of all provided parameters. Instead, do a custom thing in each service according to its capabilities.
Expand the list of known params. Introduce the difference between obligatory params and known, but not obligatory.
Remove warning.

@NikolayRys NikolayRys self-assigned this Apr 19, 2020
@NikolayRys
Copy link
Owner Author

@vitkarpov Could you please share your opinion on this? I'm afraid that somebody might be using this to achieve something.
Like so:
<div class="twitter" data-hello="goodbye">Tweet</div>
that changes URL to:
https://twitter.com/intent/tweet?url={url}&text={title}&hello=goodbye

@NikolayRys NikolayRys changed the title Improve parameters interpretation Improve interpretation of attributes Apr 20, 2020
@vitkarpov
Copy link
Collaborator

vitkarpov commented Apr 20, 2020

It all makes sense to me. Let's implement warnings in 2.5 and see who's going to come 😄(probably nobody but that's okay).

This was referenced Apr 29, 2020
@NikolayRys NikolayRys assigned NikolayRys and unassigned NikolayRys Apr 30, 2020
@NikolayRys NikolayRys added the 3.0 Roadmap for the next version label May 24, 2020
@NikolayRys
Copy link
Owner Author

Have been fixed in PRs above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.0 Roadmap for the next version
Projects
None yet
Development

No branches or pull requests

2 participants