Skip to content
This repository was archived by the owner on Dec 18, 2024. It is now read-only.

fix: create proper fragment urls #329

Merged
merged 1 commit into from
Nov 28, 2017

Conversation

devversion
Copy link
Member

By default fragment links are referring to the root page and the actual hash won't matter anymore. This can be fixed in a similar way to the header-link directive by updating the anchor URLs to include the current base URL.

Since those are normal links and aren't header-links, the header-link directive is not an option. A directive that updates the links accordingly would be perfect, but there is no way to interpolate directives on those HTML templates dynamically.

Having a component that creates/wraps the anchor elements is wrong and adds more pain than it helps. The links should can be just updated using simple DOM manipulation.

Fixes angular/components#8503

By default fragment links are referring to the root page and the actual hash won't matter anymore. This can be fixed in a similar way to the `header-link` directive by updating the anchor URLs to include the current base URL.

Since those are normal links and aren't `header-links`, the `header-link` directive is not an option. A directive that updates the links accordingly would be perfect, but there is no way to interpolate directives on those HTML templates dynamically.

Having a component that creates/wraps the anchor elements is wrong and adds more pain than it helps. The links should can be just updated using simple DOM manipulation.
Copy link
Contributor

@amcdnl amcdnl left a comment

Choose a reason for hiding this comment

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

LGTM

@devversion
Copy link
Member Author

devversion commented Nov 28, 2017

cc. @jelbourn (Related angular/components#8660)

@jelbourn jelbourn merged commit 89a6cf1 into angular:master Nov 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation link
4 participants