Skip to content

Add scroll marker using fake scrollbar implementation#436

Open
arogl wants to merge 9 commits into
brandon1024:developfrom
arogl:scollbar-highlight
Open

Add scroll marker using fake scrollbar implementation#436
arogl wants to merge 9 commits into
brandon1024:developfrom
arogl:scollbar-highlight

Conversation

@arogl

@arogl arogl commented Mar 28, 2026

Copy link
Copy Markdown

Fixes

#368

Changes Proposed in this Pull Request:

  • Add scollbar marker to options
  • Show markers on scrollbar

Additional Comments and Documentation:

Produced with the help of claude

@pzhlkj6612

Copy link
Copy Markdown
Contributor

interesting! do you have screenshots?

@arogl

arogl commented Mar 28, 2026

Copy link
Copy Markdown
Author

Options screen
image

No Matches
image

Matches on news.com.au

image

It has light and dark scrollbar capability, but not extensively tested

follow existing style.
- Can put "enable" or "disable" in description instead of messages.
- Can omit "_enabled" from js code to shorten code lines.

@pzhlkj6612 pzhlkj6612 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code and L10N strings in this PR don't seem to be consistent with the existing style and design, and there are some useless if-else and try-catch statements.

I've modified your changes in arogl#1. PLease check. The code in "content/highlighter.js" needs more cleanup and I'm working on it.

You can directly merge that PR if it looks good to you. The merge commit can be preserved, and you don't need to force-push. The maintainer will squash all commits during PR merging.

I believe that keeping things consistent will help future developers. I also believe that we will compose some linter rules for this project then.

@pzhlkj6612

Copy link
Copy Markdown
Contributor

Thank you for merging the code.

Now I'm continuing working on it. I think all markers in the virtual scrollbar should be added and removed simultaneously with the addition or removal of occurrences on the page, then the procedure will look more tidy and more integrated.

@brandon1024

Copy link
Copy Markdown
Owner

Let me know when these changes are ready and I’ll have a final review 👍

@arogl

arogl commented May 1, 2026

Copy link
Copy Markdown
Author

Sorry forgot about this one.

@pzhlkj6612

Copy link
Copy Markdown
Contributor

Thanks. I'm working on removing all try-catch blocks these days. The code in this PR looks to be against the existing code style.

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.

3 participants