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(proxy): simplify URL construction in pagination links (#36) #37

Merged

Conversation

sikatikenmogne
Copy link
Contributor

@sikatikenmogne sikatikenmogne commented Aug 26, 2024

Related Issues

Changes (initial)

  • Added a new helper function called removeDomain to handle URL parsing and remove the domain from the given full URL.
  • Updated the constructUrl helper function to use removeDomain for constructing clean URLs.
  • Modified the pagination.hbs template to use removeDomain in the href attribute of the page links.

After review

  • Simplifying URL construction by removing @root.fullUrl host part

Preview

2024-08-26-16-59-42.mp4

Tested environments

  • Github Codespaces
  • Windows
  • Microsoft DevContainer

@sikatikenmogne
Copy link
Contributor Author

Hi @pythonbrad, thanks' for your review. I've applied your suggestion and reverted my commit about the helper function.
If you decide not to merge this branch, I’ve also opened a separate PR (#38) that addresses only the suggested changes. I hope you will appreciate it.

@pythonbrad
Copy link
Member

pythonbrad commented Aug 26, 2024

Ok good, when I will test it, I will inform you.

@pythonbrad
Copy link
Member

@sikatikenmogne, it's not good to push more than one pr who solve the same issue.

@sikatikenmogne
Copy link
Contributor Author

sikatikenmogne commented Aug 26, 2024

@sikatikenmogne, it's not good to push more than one pr who solve the same issue.

I know, I just thought it would be better to summarize the changes in this conversation in a one commit pull request instead of a 3 which looks a bit messy, feel free to close the pull request which seems like too much.

@sikatikenmogne
Copy link
Contributor Author

I've closed the PR #38

@sikatikenmogne sikatikenmogne changed the title feat: add helper function for constructing clean URLs in pagination component fix(proxy): simplify URL construction in pagination links (#36) Aug 27, 2024
@sikatikenmogne
Copy link
Contributor Author

@rakici

Copy link
Contributor

@rakici rakici left a comment

Choose a reason for hiding this comment

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

Nice catch.

@rakici rakici merged commit 6a14578 into osscameroon:main Sep 5, 2024
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.

Issue with proxy forwarder
3 participants