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.

Sign up for GitHub

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

Jump to bottom

Make author names in forum quotes clickable #7364

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Make author names in forum quotes clickable #7364

wants to merge 17 commits into from

Conversation

just-a-hriday
Copy link
Contributor

@just-a-hriday just-a-hriday commented Apr 20, 2024 •
edited

Resolves #7363

Changes

Adds an addon which replaces all valid usernames in forum quotes with clickable links.

clickable-quote-authors

The addon also has an option for hidden links.

clickable-quote-authors_hidden-link

Tests

Tested in Arc for Windows 0.18.0 and Chrome 123.
I don't have firefox, can someone else test?

This addon can make links to non-existent users.
Does anyone know a way to check if a user exists?

@just-a-hriday
Copy link
Contributor Author

Github says there are conflicts in addons.json, I have no idea what it's talking about.

@DNin01
Copy link
Member

DNin01 commented Apr 20, 2024

Github says there are conflicts in addons.json, I have no idea what it's talking about.

Would you mind if I get this branch up to date with upstream/master (our repository) and resolve the conflict in the process? It'll update this branch to the latest source code commit, which is currently on v1.38.0-prerelease. After the merge, all you'll need to do is pull the changes to your computer.

In the future, remember to base your branch off of upstream/master so you're all up to date.

@just-a-hriday
Copy link
Contributor Author

Would you mind if I get this branch up to date with upstream/master (our repository) and resolve the conflict in the process? It'll update this branch to the latest source code commit, which is currently on v1.38.0-prerelease. After the merge, all you'll need to do is pull the changes to your computer.

In the future, remember to base your branch off of upstream/master so you're all up to date.

Yeah, please do that. Thanks!

@DNin01
Copy link
Member

DNin01 commented Apr 21, 2024 •
edited

Alright, you're all set!

@just-a-hriday
Copy link
Contributor Author

@WorldLanguages can you review?

Samq64
Samq64 reviewed Apr 23, 2024
View reviewed changes
Copy link
Member

@Samq64 Samq64 left a comment

Choose a reason for hiding this comment

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

The addon isn't dynamic but it works fine.

This will need a little more discussion though since it's very similar to curator-link and #4845.

addons/clickable-quote-authors/addon.json Outdated Show resolved Hide resolved
addons/clickable-quote-authors/addon.json Outdated Show resolved Hide resolved
addons/clickable-quote-authors/userscript.js Outdated Show resolved Hide resolved
addons/clickable-quote-authors/userscript.js Outdated
const authors = document.getElementsByClassName("bb-quote-author");

for (const author of authors) {
const authorName = author.textContent.match(/(?<=^)[\w-]{2,30}(?= wrote:$)/);
Copy link
Member

Choose a reason for hiding this comment

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

Would it be worth making an API request to check if the user actually exists?

Copy link
Member

Choose a reason for hiding this comment

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

no
but you could add "TOLORS" to a blacklist if you want

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if someone quotes the actual TOLORS user?

Copy link
Contributor

Choose a reason for hiding this comment

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

Would make it inconsistent.

@Samq64 Samq64 added type: enhancement New feature for the project new addon Related to new addons to this extension. `scope: addons` should still be added. scope: addon Related to one or multiple addons status: needs discussion Still in review or consideration labels Apr 23, 2024
@WorldLanguages
Copy link
Member

Sorry, I have limited availability to review addon PRs at the moment. But the code looks alright.

@OneShot-Niko

This comment has been minimized.

Sign in to view
@Samq64 Samq64 removed the status: needs discussion Still in review or consideration label Apr 23, 2024
@just-a-hriday
Copy link
Contributor Author

The hidden link setting works dynamically now.

just-a-hriday and others added 2 commits April 25, 2024 13:32
@just-a-hriday @Samq64
rename to "Clickable forum quote authors"
4f31231 
Co-authored-by: Samq64 <81489795+Samq64@users.noreply.github.com>
@just-a-hriday
rename hidden link setting
d6fae26
Samq64
Samq64 approved these changes Apr 28, 2024
View reviewed changes
Copy link
Member

@Samq64 Samq64 left a comment

Choose a reason for hiding this comment

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

Seems fine as its own addon and dynamic disable isn't a requirement.

addons/clickable-quote-authors/addon.json Outdated Show resolved Hide resolved
addons/clickable-quote-authors/addon.json Show resolved Hide resolved
@Samq64 Samq64 added the status: needs review [Use when there's already 1 approval] Review needed on the PR label Apr 28, 2024
just-a-hriday and others added 3 commits April 29, 2024 07:03
@just-a-hriday @Samq64
remove trailing space in addon name
7c52aab 
Co-authored-by: Samq64 <81489795+Samq64@users.noreply.github.com>
@just-a-hriday @Samq64
add dynamicEnable to manifest
9ee90d9 
Co-authored-by: Samq64 <81489795+Samq64@users.noreply.github.com>
@just-a-hriday
add dynamic disable
eb6aac9
@just-a-hriday
Copy link
Contributor Author

I've added dynamic disable.

@just-a-hriday
Copy link
Contributor Author

Can someone please review this PR?

Samq64
Samq64 approved these changes May 12, 2024
View reviewed changes
Copy link
Member

@Samq64 Samq64 left a comment

Choose a reason for hiding this comment

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

I still think this would be better as part of curator-link though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Secret-chest Secret-chest Secret-chest left review comments

@Samq64 Samq64 Samq64 approved these changes

@WorldLanguages WorldLanguages Awaiting requested review from WorldLanguages WorldLanguages is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
new addon Related to new addons to this extension. `scope: addons` should still be added. scope: addon Related to one or multiple addons status: needs review [Use when there's already 1 approval] Review needed on the PR type: enhancement New feature for the project
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make author names in forum quotes clickable
6 participants
@just-a-hriday @DNin01 @WorldLanguages @OneShot-Niko @Secret-chest @Samq64