Skip to content

Conversation

fabianwolski
Copy link
Contributor

useLink() hook
Deleted DocLinkCreator enitrely.
Using FrontendConfig directly

Current implementation only handles documentationBaseUrl. How can we implement githubUrl - the github issues link for example? To test currently please add const { documentationHomepage } = useLink(); to login.tsx

@fabianwolski fabianwolski changed the title DocumentationUrl ONLY implementation Refactor: Rework DocLinkCreator class May 2, 2025
andreaskienle
andreaskienle previously approved these changes May 7, 2025
Copy link
Contributor

@andreaskienle andreaskienle left a comment

Choose a reason for hiding this comment

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

Good job! 👍🏻

I’ve added two small suggestions that should be quick to address.

@andreaskienle andreaskienle self-assigned this May 7, 2025
Copy link
Contributor

@lucasgoral lucasgoral left a comment

Choose a reason for hiding this comment

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

Looks good to me. I only wonder if we should deconstruct the object like this
const { githubIssuesSupportTicket } = useLink();

Maybe it is better to not deconstruct it but use it like this:
const links = useLink();
and then { links.githubIssuesSupportTicket }

but this is only matter of style

@andreaskienle
Copy link
Contributor

Looks good to me. I only wonder if we should deconstruct the object like this const { githubIssuesSupportTicket } = useLink();

Maybe it is better to not deconstruct it but use it like this: const links = useLink(); and then { links.githubIssuesSupportTicket }

but this is only matter of style

Would also be a valid approach! It's implemented as specified in the backlog item. Since we'll be revisiting the whole topic of links soon anyway, we can always re-evaluate then. 🙂

@andreaskienle andreaskienle merged commit 1f774a9 into main May 16, 2025
4 checks passed
@andreaskienle andreaskienle deleted the refactor/rework-doclinkcreator-class branch May 16, 2025 08:29
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