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

Fix context menu handling to prioritize link URLs and include anchor text as bookmark titles #866

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dakshpareek
Copy link
Contributor

Problem: When users select anchor text and click "Add to Hoarder," the extension was capturing the selected text instead of the link URL. As a result, only the selected text was saved in Hoarder.

issue

Solution: Modified handleContextMenuClick in background.ts:

  • Prioritized info.linkUrl over info.selectionText when determining what to hoard.
  • Captured the selected anchor text as the title when saving a link bookmark.

solution

- Modify  to prioritize  when saving bookmarks
- Include selected anchor text as the bookmark title when saving links
url: info.linkUrl,
title: info.selectionText ?? undefined,
};
} else if (info.selectionText) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmmm, the ordering of the ifs makes me think that having text selection taking higher priority was some intentional decision. I'll need to remember why it was done that way :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha. Take your time. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any update on this?

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