ajnart / homarr

Customizable browser's home page to interact with your homeserver's Docker containers (e.g. Sonarr/Radarr)
https://homarr.dev
MIT License
5.45k stars 252 forks source link

Add `[homarr_base]` replacement for external urls #2024

Closed j3lte closed 1 month ago

j3lte commented 2 months ago

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.

Screenshot

Screenshot 2024-04-25 at 09 17 21
ajnart commented 2 months ago

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 🤣

j3lte commented 2 months ago

@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 commented 2 months ago

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

I'll update the description above

ajnart commented 2 months ago

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 commented 2 months ago

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

ajnart commented 2 months ago

@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 commented 2 months ago

@ajnart something like this?

Screenshot 2024-04-25 at 10 20 13
ajnart commented 2 months ago

@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 commented 2 months ago

~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 commented 2 months ago

@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 commented 2 months ago

~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 commented 2 months ago

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

SeDemal commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

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
j3lte commented 1 month ago

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 👍

Meierschlumpf commented 1 month ago

Maybe we should also add the url parsing to the Category open all feature: https://github.com/ajnart/homarr/blob/6a5836f096339f7f454d548bc7733b4c460b8c03/src/components/Dashboard/Wrappers/Category/Category.tsx#L47

j3lte commented 1 month ago

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

https://github.com/ajnart/homarr/blob/6a5836f096339f7f454d548bc7733b4c460b8c03/src/components/Dashboard/Wrappers/Category/Category.tsx#L47

@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 commented 1 month ago

Maybe we should also add the url parsing to the Category open all feature: https://github.com/ajnart/homarr/blob/6a5836f096339f7f454d548bc7733b4c460b8c03/src/components/Dashboard/Wrappers/Category/Category.tsx#L47

@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 commented 1 month ago

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