Skip to content
This repository was archived by the owner on Jun 24, 2025. It is now read-only.

fix(link): Allow external links containing # to open#2117

Merged
eliandoran merged 3 commits intodevelopfrom
external_links
Jun 4, 2025
Merged

fix(link): Allow external links containing # to open#2117
eliandoran merged 3 commits intodevelopfrom
external_links

Conversation

@SiriusXT
Copy link
Copy Markdown
Member

@SiriusXT SiriusXT commented Jun 3, 2025

@SiriusXT SiriusXT changed the title fix(link): Allow external links containing # to navigate fix(link): Allow external links containing # to open Jun 3, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 3, 2025

Folder/File Previous size New size Difference
/upload/TriliumNextNotes-Server-2117-merge-linux-x64.tar.xz 67.14MB +67.14MB (+100.00%)
TOTAL +67.14MB

eliandoran
eliandoran suggested changes Jun 3, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@eliandoran eliandoran left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Could you please add a test case in link.spec.ts?

As always, it needs to fail without your implementation and pass after it.

@eliandoran eliandoran marked this pull request as draft June 3, 2025 15:40
@SiriusXT
Copy link
Copy Markdown
Member Author

SiriusXT commented Jun 4, 2025

Could you please add a test case in link.spec.ts?

As always, it needs to fail without your implementation and pass after it.

@eliandoran Oops… I just ran pnpm vitest run src/services/link.spec.ts and, well… it didn't go quite as planned.

 RUN  v3.2.1 X:/workspace/others/trilium/Notes/apps/client

──────────────────────────────────────────────────────
                  Failed Suites: 1
──────────────────────────────────────────────────────

 FAIL  src/services/link.spec.ts [ src/services/link.spec.ts ]
ReferenceError: document is not defined
 ❯ enableDismissTrigger ../../node_modules/bootstrap/js/src/util/component-functions.js:16:19
 ❯ ../../node_modules/bootstrap/js/src/alert.js:79:1
 ❯ ../../node_modules/bootstrap/dist/js/bootstrap.js:7:83
 ❯ Object.<anonymous> ../../node_modules/bootstrap/dist/js/bootstrap.js:10:3

I have no idea how to deal with this at the moment.

@SiriusXT
Copy link
Copy Markdown
Member Author

SiriusXT commented Jun 4, 2025

I temporarily solved it by putting the function directly into link.spec.ts

@SiriusXT SiriusXT marked this pull request as ready for review June 4, 2025 07:12
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jun 4, 2025
eliandoran
eliandoran approved these changes Jun 4, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@eliandoran eliandoran left a comment

Choose a reason for hiding this comment

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

LGTM.

@eliandoran eliandoran added this to the v0.95.0 milestone Jun 4, 2025
@eliandoran eliandoran merged commit aa6ad0b into develop Jun 4, 2025
11 checks passed
@eliandoran eliandoran deleted the external_links branch June 4, 2025 08:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@eliandoran eliandoran eliandoran approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

v0.94.1

Development

Successfully merging this pull request may close these issues.

External links using fragment navigation are wrongly recognized as internal links

2 participants

@SiriusXT @eliandoran