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

Add Nitter redirection support #231

Merged
merged 7 commits into from
Sep 8, 2024
Merged

Add Nitter redirection support #231

merged 7 commits into from
Sep 8, 2024

Conversation

MarshDeer
Copy link

I'm opening this draft PR very early in the process so that I can share my work for transparency and cooperation purposes.

I've begun working on support for more preferences, starting with the option to make vx redirect into a chosen Nitter instance instead of X/Twitter. This one goes for those of us who are exhausted of people sending links we can't read because we don't have an account.

The user will query the status.d420.de API and get a list of healthy instances they can use. Their selected instance will get saved in localStorage, and openLink() will check this preference and act accordingly.

This feature will be incompatible with the "Open in app" function, but users who have the app installed probably don't need or want to use Nitter anyways.

LibRedirect and similar browser extensions might be a better option for this? It could still fit someone's use case, I guess.

Will develop properly later, this commit is mostly just there so I can open the draft PR
@dylanpdx
Copy link
Owner

dylanpdx commented Sep 6, 2024

This feature will be incompatible with the "Open in app" function
Might be best to disable either option when the other is checked as well, so there's less confusion

@MarshDeer
Copy link
Author

Might be best to disable either option when the other is checked as well, so there's less confusion

Shouldn't cause any problems in practice, really, since the "open in app" setting would trigger before there's a chance for the user to be redirected to nitter.

It would be a good idea to check for conflicts between preferences, though. I'll add a quick "is this incompatible with anything else" check later

@MarshDeer
Copy link
Author

I propose maybe renaming openInApp.js to openTweet.js or redirect.js or something similar since its functions will now go beyond opening tweets in the app? Desktop-only users who will never see the app go through it, after all

That aside, I think everything should be functional now! I'll rest for a bit and reread the entire PR later to make sure, but I'm knocking on wood and marking it as ready for review

@MarshDeer MarshDeer marked this pull request as ready for review September 7, 2024 00:44
Comment on lines +28 to +42
for (const instance of data.hosts) {
if (instance.points > 0) {
let labelOpen = `<label for="instance${i}">`;
let labelClose = `</label>`;
let row = document.createElement("tr");
let input = document.createElement("td");
input.innerHTML = `${labelOpen} <input type="radio" name="instance" id="instance${i}" value="${instance.url}" onclick="savePreference('frontendUrl', this.value)"> ${labelClose}`;
let name = document.createElement("td");
name.innerHTML = `${labelOpen} ${instance.domain} ${labelClose}`
let health = document.createElement("td");
health.innerHTML = `${labelOpen} ${instance.healthy} ${labelClose}`;
let score = document.createElement("td");
score.innerHTML = `${labelOpen} ${instance.points} ${labelClose}`;
row.append(input, name, health, score);
instanceList.append(row);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love the way I assembled each <td> with innerHTML.

There's probably a more elegant way to handle it. That said: this solution works well enough, it's readable, and it's not performance-heavy enough to need optimizing.

I don't clear #instanceList at fetch time. In theory, this could create duplicates if someone requests instances more than once, but the API's rate limit makes that very unlikely to happen unless someone is specifically trying to make it fail.

static/style.css Outdated
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire file is extremely messy. It should work, but it could be a good idea to run it through a linter or something just in case

@dylanpdx
Copy link
Owner

dylanpdx commented Sep 7, 2024

Getting an error on test:

Uncaught ReferenceError: frontendUrl is not defined
    openTweet /openInApp.js:25

I think you might need to assign localStorage.getItem("frontendUrl") to a variable named frontendUrl so that it can be used in the redirect
Or you could also do

window.location = `${localStorage.getItem("frontendUrl")}/i/status/${tweetId}`

I'm fine with both options!

@MarshDeer
Copy link
Author

Whoops. Knew I was gonna miss something dumb like that. On it~

@dylanpdx dylanpdx merged commit 6e3e742 into dylanpdx:main Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants