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

Add [homarr_base] replacement for external urls #2024

Merged
merged 19 commits into from
May 7, 2024

Conversation

j3lte
Copy link
Contributor

@j3lte j3lte commented Apr 22, 2024

Category

Feature

Overview

This introduces a way of crafting a dynamic url purely client-side. By starting your url with [homarr_base] it replaces this string with the current base url (client-side).

Why?

I am using Homarr myself as a dashboard for my home server.

  • When I am outside of the network, homarr is available as http://home-server-remote (through Tailscale), where all other services are running on different ports.
  • When I am at home, it is internally reachable as http://home-server.
  • Currently I have a duplicate row of all my services due to this naming scheme

Screenshot

Screenshot 2024-04-25 at 09 17 21

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hi 👋. Thank you for making your first contribution to Homarr. Please ensure that you've completed all the points in the TODO checklist. We'll review your changes shortly.

@ajnart
Copy link
Owner

ajnart commented Apr 22, 2024

Thanks for this PR @j3lte ! I'm looking at it 👀
How did you know that ://[homarr_domain] will by replaced by the current domain ? I will try to test this PR but I'm curious where you learned that. Do you know about any limitations on this feature (Browser, OS ?)

Edit: Nevermind I didn't properly understand the code at first sight 🤣

Copy link
Owner

@ajnart ajnart 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 ! Didn't get the time to test it on my own domain tho

Browser compatibility looks good too 👍🏼
https://developer.mozilla.org/en-US/docs/Web/API/Location/hostname#browser_compatibility

Copy link
Collaborator

@SeDemal SeDemal left a comment

Choose a reason for hiding this comment

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

There may be cases where the internal is http while the outside is https, so maybe this should also be an option?
[homarr_protocol]://[homarr_domain]
or directly [homarr_baseUrl]

@@ -26,6 +26,15 @@ export const AppTile = ({ className, app }: AppTileProps) => {
.join(': ');

const isRow = app.appearance.positionAppName.includes('row');
const externalUrl = useMemo(() => {
if (app.behaviour.externalUrl.length > 0) {
if (app.behaviour.externalUrl.includes('://[homarr_domain]') && window.location.hostname) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't work with subdomains if I read that correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you give me an example?

Copy link
Owner

@ajnart ajnart Apr 25, 2024

Choose a reason for hiding this comment

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

Example homarr install on a subdomain would be homarr.domain.com

are you able to test that ? @j3lte

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, let's imagine homarr has "https://homarr.domain.com" and "https://homarr.localdomain.com", and sonarr is on "https://sonarr.domain.com" and "https://sonarr.localdomain.com", each being there depending if you're home or external.
You would then do "https://sonarr.[homarr_domain]" in the config.
They share domain but not the subdomain, and the subdomain is separating the domain from the "://", so testing for it means subdomains won't work. At least that's how I interpret it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, unfortunately subdomain is not part of window.location. So in the case of "homar.domain.com", window.location.host and window.location.hostname are both "homar.domain.com", not "domain.com". Your proposal with both protocol and domain separated would work if homarr is at "domain.com" and sonar at "sonar.domain.com"

Copy link
Collaborator

@SeDemal SeDemal Apr 26, 2024

Choose a reason for hiding this comment

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

[homarr_hostname] -> returns homarr's full base url including it's current subdomain
[homarr_domain] -> returns homarr's domain with subdomain filtered out
[homarr_protocol] -> return homarr's protocol (http/s)
[homarr_baseUrl] -> full address excluding port and path

You could manually filter out subdomains, but I agree it could be a bit of a pain.
I found this package that does it for you, and quite well at that: https://www.npmjs.com/package/tldts
It adds a new dependency but frankly we could use it to better format our URL's in integrations so it's fine.

I'm sorry if it may feel like I'm nagging for contributions, I just feel like your idea has that much potential and should be implemented to it's fullest.

If you want you can come by our discord to make things easier.

@j3lte
Copy link
Contributor Author

j3lte commented Apr 25, 2024

@SeDemal I don't think the internal one changes based on the external? As far as I know the only URL that is exposed is the external url.

I did leave out the protocol, only ://[homar_domain] is replaced (thus you can write http or https anyway)

Edit: @SeDemal you are absolutely right, I didn't think about that.... This should include the protocol.

@j3lte
Copy link
Contributor Author

j3lte commented Apr 25, 2024

@ajnart based on the comment from @SeDemal I updated the functionality. It now includes the protocol...

  • You can now set the external url to [homarr_base] and it will replace it with the current domain AND protocol.

I'll update the description above

@j3lte j3lte changed the title Add ://[homarr_domain] replacement for external urls Add [homarr_base] replacement for external urls Apr 25, 2024
@ajnart
Copy link
Owner

ajnart commented Apr 25, 2024

Great! Can you make a PR on the docs repository so that people can learn about this ? We'd probably want to include it in the releases notes as well.

you could also add a little [i] tip component, it should be easy to find how to do it in the code

@j3lte
Copy link
Contributor Author

j3lte commented Apr 25, 2024

@ajnart will add a tooltip first, then the docs :) 👍

@ajnart
Copy link
Owner

ajnart commented Apr 25, 2024

@ajnart will add a tooltip first, then the docs :) 👍

Not a tooltip, a tip is a component we made to display tips about advanced functionality someone wouldn't normally know about

@j3lte
Copy link
Contributor Author

j3lte commented Apr 25, 2024

@ajnart something like this?

Screenshot 2024-04-25 at 10 20 13

@ajnart
Copy link
Owner

ajnart commented Apr 25, 2024

@ajnart something like this?

Screenshot 2024-04-25 at 10 20 13

Perfect position 🤩
Maybe add an example at the bottom
Don't forget to make it a translation for accessibility purposes

@j3lte
Copy link
Contributor Author

j3lte commented Apr 25, 2024

It does use the translations, do I need to translate it myself for all the other languages? (I'll probably have to use Google Translate or something 😬)

Edit: added the translations

@SeDemal
Copy link
Collaborator

SeDemal commented Apr 25, 2024

@SeDemal I don't think the internal one changes based on the external? As far as I know the only URL that is exposed is the external url.

I did leave out the protocol, only ://[homar_domain] is replaced (thus you can write http or https anyway)

Edit: @SeDemal you are absolutely right, I didn't think about that.... This should include the protocol.

My mistake, I said internal but wasn't talking about the internal address. I was talking about the address the user uses at home VS outside.

@ajnart based on the comment from @SeDemal I updated the functionality. It now includes the protocol...

  • You can now set the external url to [homarr_base] and it will replace it with the current domain AND protocol.

I'll update the description above

[homarr_base] is good, but you don't have to limit to one of them.
homarr_base is a combo with protocol and url, the older one homarr_domain with just the url and finally homarr_protocol with http/s.
Having all 3 allows simplicity in some cases and subdomain use in others.

@SeDemal
Copy link
Collaborator

SeDemal commented Apr 25, 2024

It does use the translations, do I need to translate it myself for all the other languages? (I'll probably have to use Google Translate or something 😬)

Edit: added the translations

Sorry you already took the time to do it, But never ever add another language than english, it breaks the system.
Please revert and only keep English.

@j3lte
Copy link
Contributor Author

j3lte commented Apr 25, 2024

Sorry, how does this break the system? The keys are there per language, I just added them to each of them...

@SeDemal
Copy link
Collaborator

SeDemal commented Apr 25, 2024

It doesn't break homarr, it breaks with the translation platform.
Also, we have proofreaders that need to double check translations, we can't try and sneak translations in like that.

@j3lte
Copy link
Contributor Author

j3lte commented Apr 25, 2024

@SeDemal done. It might be a good idea to document it somewhere (for example in the contribution guide) that you can't add translations.

@SeDemal
Copy link
Collaborator

SeDemal commented Apr 26, 2024

@SeDemal done. It might be a good idea to document it somewhere (for example in the contribution guide) that you can't add translations.

That's a fair point.
Tbh I made the same mistake on one of my first contributions too.

@j3lte
Copy link
Contributor Author

j3lte commented Apr 27, 2024

Updated the functionality based on feedback from @SeDemal. I'm not completely satisfied with the tooltip (it contains a lot of text), but it does work as expected. Here's the updated functionality:

Screenshot 2024-04-27 at 12 11 31

Copy link
Collaborator

@Meierschlumpf Meierschlumpf left a comment

Choose a reason for hiding this comment

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

Can you revert your changes to .vscode/settings.json and data/configs/default.json?
Beside this this looks awesome ❤️‍🔥, thanks for your contribution 😃

@j3lte
Copy link
Contributor Author

j3lte commented May 2, 2024

Can you revert your changes to .vscode/settings.json and data/configs/default.json? Beside this this looks awesome ❤️‍🔥, thanks for your contribution 😃

@Meierschlumpf didn't notice I committed those changes as well... I reverted these 👍

Copy link
Collaborator

@Meierschlumpf Meierschlumpf left a comment

Choose a reason for hiding this comment

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

lgtm, @SeDemal for you as well?

Copy link
Collaborator

@SeDemal SeDemal left a comment

Choose a reason for hiding this comment

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

Comments are not that important, so you can disregard them if you really want.
Everything looks good now otherwise, great work.

src/components/Dashboard/Tiles/Apps/AppTile.tsx Outdated Show resolved Hide resolved
src/components/Dashboard/Tiles/Apps/AppTile.tsx Outdated Show resolved Hide resolved
j3lte and others added 2 commits May 6, 2024 13:26
@Meierschlumpf
Copy link
Collaborator

Maybe we should also add the url parsing to the Category open all feature:

const popUp = window.open(app.url, app.id);

@j3lte
Copy link
Contributor Author

j3lte commented May 6, 2024

Maybe we should also add the url parsing to the Category open all feature:

const popUp = window.open(app.url, app.id);

@Meierschlumpf this probably needs a bit more testing. Currently it's using the internal app.url instead of the app.externalUrl. We could implement it there, but we should then also rewrite it to open the external url instead of the internal one...

@Meierschlumpf
Copy link
Collaborator

Maybe we should also add the url parsing to the Category open all feature:

const popUp = window.open(app.url, app.id);

@Meierschlumpf this probably needs a bit more testing. Currently it's using the internal app.url instead of the app.externalUrl. We could implement it there, but we should then also rewrite it to open the external url instead of the internal one...

Yes I saw there was an issue today about the internal / external url stuff. Maybe we could add a hook useParsedUrl which just returns the memoized parsed url, use that above the handleMenuClick method:
https://github.com/ajnart/homarr/blob/18cd1f961f3fc6670af4e5722fcb7bd6f5925536/src/components/Dashboard/Wrappers/Category/Category.tsx#L44C9-L44C25
and then add another method getExternalUrl which accepts app and the parsed url as arguments. Then this getter can be used within a useMemo in the AppTile and as function within the foreach loop of the handleMenuClick method.

@j3lte
Copy link
Contributor Author

j3lte commented May 7, 2024

@Meierschlumpf see last commit, I rewrote the hook a little bit and added this to Category.tsx

Copy link
Collaborator

@SeDemal SeDemal left a comment

Choose a reason for hiding this comment

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

Thanks for all the hard work, it's a great feature :)

@manuel-rw manuel-rw merged commit 5fbb444 into ajnart:dev May 7, 2024
2 checks passed
truecharts-admin added a commit to truecharts/charts that referenced this pull request May 8, 2024
…c270b55 by renovate (#21754)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [ghcr.io/ajnart/homarr](https://github.com/ajnart/homarr) | patch |
`0.15.2` -> `0.15.3` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>ajnart/homarr (ghcr.io/ajnart/homarr)</summary>

### [`v0.15.3`](https://github.com/ajnart/homarr/releases/tag/v0.15.3)

[Compare
Source](https://github.com/ajnart/homarr/compare/v0.15.2...v0.15.3)

> \[!NOTE]\
> We've been working actively on working torwards version 1.0 which will
include many improvements to performance, security and the overall look
& feel of Homarr. It will greatly overhaul the technical architecture of
Homarr. This work is done by volunteers. Please consider supporting our
work via donations at https://opencollective.com/homarr

#### Support for generic Home Assistant switches (lights, fans, ...)

Want to toggle your kitchen lights or shut off the music? Using the
brand new support for generic switches you can toggle now almost
anything:

![homeassistant-lights](https://github.com/ajnart/homarr/assets/30572287/2cd79222-ca7d-4773-95e8-845cb382468c)

#### Promox integration

Homarr now features a Promox widget that can display CPU, RAM and more
information directly on your dashboard:

![305290159-b6ed7b8a-335b-41d0-b5c8-8edb736db3ee](https://github.com/ajnart/homarr/assets/30572287/cfc6f949-16f8-4add-a4eb-668c258c3305)

#### Tdarr widget

Homarr now also integrates with Tdarr to display the status of your
workers and the queue:

![Untitled](https://github.com/ajnart/homarr/assets/30572287/b368c4ae-7b6e-42d7-9bfc-58a110646dbe)

#### Add placeholder for dynamic URLs

When using advanced reverse proxy setups, sometimes the hostname is not
known yet.
We have added a new feature that enables you to replace certain
placeholders: <img width="598"
alt="326171056-a55c3e49-9cd6-4523-90cf-16e91b2c7502"
src="https://github.com/ajnart/homarr/assets/30572287/53cf7469-586a-4fda-9dbf-3651cf3e2189">

#### Usability improvements to the torrent widget

The torrent widget has received a major upgrade and should scale now
better on most devices with improved customizability:


![image](https://github.com/ajnart/homarr/assets/30572287/8dbcd786-03ed-49da-b4ef-8a7b5b5bdd46)

![image](https://github.com/ajnart/homarr/assets/26098587/325ec664-c0cc-4cfa-bfb5-717d3674bda4)

#### Other bugfixes

-   Fixed overlapping of the grid elements and the navbar
-   Fixed video playback for iOS and Apple devices
- Fixed issues with the certificates for the avatars (this will also
resolve issues with the page being shown as "not secure")

#### What's Changed

- fix: health monitoring hotfix by
[@&#8203;hillaliy](https://github.com/hillaliy) in
[ajnart/homarr#1970
- fix: change media request TV poster and movie name by
[@&#8203;SeDemal](https://github.com/SeDemal) in
[ajnart/homarr#1983
- fix: weather widget does not refresh automatically by
[@&#8203;hillaliy](https://github.com/hillaliy) in
[ajnart/homarr#1981
- fix: [#&#8203;1976](https://github.com/ajnart/homarr/issues/1976)
lower debounce time by
[@&#8203;manuel-rw](https://github.com/manuel-rw) in
[ajnart/homarr#1979
- chore: new Crowdin updates by
[@&#8203;ajnart](https://github.com/ajnart) in
[ajnart/homarr#1968
- feat: add Proxmox integration/widget by
[@&#8203;dslatt](https://github.com/dslatt) in
[ajnart/homarr#1903
- feat: columns customize by
[@&#8203;hillaliy](https://github.com/hillaliy) in
[ajnart/homarr#1975
- feat: added playsInline to VideoBackground by
[@&#8203;spkesDE](https://github.com/spkesDE) in
[ajnart/homarr#1996
- fix: ping indicators floating above header in board customization page
by [@&#8203;krishnamuppaneni](https://github.com/krishnamuppaneni) in
[ajnart/homarr#1998
- fix: OIDC Timeout by
[@&#8203;catrielmuller](https://github.com/catrielmuller) in
[ajnart/homarr#2002
- fix: Pass axios error into log call for proxmox. Please include in
v0.15.3 by [@&#8203;dslatt](https://github.com/dslatt) in
[ajnart/homarr#2012
- feat: add romanian language support by
[@&#8203;Meierschlumpf](https://github.com/Meierschlumpf) in
[ajnart/homarr#2017
- fix: missing romanian language in next-i18next.config.js by
[@&#8203;Meierschlumpf](https://github.com/Meierschlumpf) in
[ajnart/homarr#2018
- feat: add Tdarr integration and widget by
[@&#8203;jbruell](https://github.com/jbruell) in
[ajnart/homarr#1882
- feat: torrent widget: polishing UI and improve popover interactions by
[@&#8203;SeDemal](https://github.com/SeDemal) in
[ajnart/homarr#2016
- fix: Modals titles nested headers and edit mode nested buttons errors
by [@&#8203;SeDemal](https://github.com/SeDemal) in
[ajnart/homarr#2019
- fix: Avatar host by [@&#8203;SeDemal](https://github.com/SeDemal) in
[ajnart/homarr#2027
- feat: Home Assistant entity generic toggle by
[@&#8203;tuggan](https://github.com/tuggan) in
[ajnart/homarr#2015
- feat: add logout callback URL and session expiration environment
variables by [@&#8203;SeDemal](https://github.com/SeDemal) in
[ajnart/homarr#2023
- fix: ldap filters by [@&#8203;SeDemal](https://github.com/SeDemal)
in
[ajnart/homarr#2033
- chore: new Crowdin updates by
[@&#8203;ajnart](https://github.com/ajnart) in
[ajnart/homarr#1984
- feat: add `[homarr_base]` replacement for external urls by
[@&#8203;j3lte](https://github.com/j3lte) in
[ajnart/homarr#2024
- core: increase version to 0.15.3 by
[@&#8203;Meierschlumpf](https://github.com/Meierschlumpf) in
[ajnart/homarr#2007

#### New Contributors

- [@&#8203;dslatt](https://github.com/dslatt) made their first
contribution in
[ajnart/homarr#1903
- [@&#8203;krishnamuppaneni](https://github.com/krishnamuppaneni) made
their first contribution in
[ajnart/homarr#1998
- [@&#8203;catrielmuller](https://github.com/catrielmuller) made their
first contribution in
[ajnart/homarr#2002
- [@&#8203;jbruell](https://github.com/jbruell) made their first
contribution in
[ajnart/homarr#1882
- [@&#8203;j3lte](https://github.com/j3lte) made their first
contribution in
[ajnart/homarr#2024

**Full Changelog**:
ajnart/homarr@v0.15.2...v0.15.3

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNDkuMyIsInVwZGF0ZWRJblZlciI6IjM3LjM0OS4zIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbImF1dG9tZXJnZSIsInVwZGF0ZS9kb2NrZXIvZ2VuZXJhbC9ub24tbWFqb3IiXX0=-->
@j3lte j3lte deleted the feature/dynamic_url branch May 8, 2024 14:12
@JoJenH
Copy link

JoJenH commented Sep 19, 2024

It's a great feature. But it doesn't seem powerful enough. I use homarr. home-hub.pub/private.example.com as homarr domain name, use *.home-hub.pub/private.example.com as other services domain name. This cannot be done with existing variables. I think that Jinja is a better choice. Or simple string manipulation. Thanks!

@j3lte
Copy link
Contributor Author

j3lte commented Sep 19, 2024

It's a great feature. But it doesn't seem powerful enough. I use homarr. home-hub.pub/private.example.com as homarr domain name, use *.home-hub.pub/private.example.com as other services domain name. This cannot be done with existing variables. I think that Jinja is a better choice. Or simple string manipulation. Thanks!

If you have a better solution, please provide a PR and maybe send this suggestion as a separate issue, instead of commenting on an old PR 😄

@JoJenH
Copy link

JoJenH commented Sep 19, 2024

It's a great feature. But it doesn't seem powerful enough. I use homarr. home-hub.pub/private.example.com as homarr domain name, use *.home-hub.pub/private.example.com as other services domain name. This cannot be done with existing variables. I think that Jinja is a better choice. Or simple string manipulation. Thanks!

If you have a better solution, please provide a PR and maybe send this suggestion as a separate issue, instead of commenting on an old PR 😄

Thank you and sorry. It's my mistake.

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.

6 participants