search

waldyrious / primerpedia

Simplified extracts of Wikipedia articles, showing just the basic information.
https://primerpedia.toolforge.org
Other
11 stars 7 forks source link

Fix permalink feature to include the full URL #56

Open waldyrious opened 6 years ago

waldyrious commented 6 years ago

The "copy permalink" feature was implemented in #42, but there seems to be an issue. Relevant quotes from #41:

if I search for a string (or load a random article) and then click the clipboard icon, what's copied is not the entire URL but just, e.g. /primerpedia/?search=John_Doe.

and

This [happens] since the function getShareableLink only uses location.pathname which of course only contains /primerpedia. It's a straight forward fix to add location.origin as well but I'm fairly certain that I tested this functionality thoroughly.

Probably wanted to append it to https://github.com/waldyrious/primerpedia/blob/8dfcbcbd06fad82ad2a486907f520b59876c2910/primerpedia.js#L112

but forgot?

and

While trying to implement this feature I found that it's not completely done with just adding window.location.origin (as it can start with // which would break the link). There needs some proper url handling (add if search is missing, change if it's available) in place.

Witia1 commented 5 years ago

In function renderSearchResult I replaced copyShareLinkInputElement.value = getShareableLink(encodedArticleTitle); to copyShareLinkInputElement.value = shareLink; It gets full url but previous page from history, not current.

waldyrious commented 5 years ago

Thanks for the information, @Witia1. I actually am considering whether having a dedicated button to copy the URL in such a minimal interface is worth it, since the functionality is already accessible otherwise.

Specifically, the URL is already updated to match the current page being viewed (since #22), so the user can either copy the URL from the address bar, on a desktop browser, or share the page using the native sharing flow on mobile.

With all due respect and appreciation to the work that went into implementing this feature, given the above I am, at the moment, inclined to remove it altogether. What do you think?

marado commented 1 year ago

I think both options are acceptable, but I'd like to suggest that a decision should be made, because, as it is, we have the worst possible behavior.

waldyrious commented 1 year ago

Given the lack of response, and in the interests of simplifying the code to be more maintainable (without significant loss of functionality, as mentioned in my last comment), I'd say the best option is to remove the feature. Would you be willing to submit a patch, @marado?