New UI: Use URL from window.location #4281
Conversation
…rather than a hard coded, and potentially wrong, placeHolder URL
…coded URL with a DEFAULT_SOLR_URL
epugh
left a comment
There was a problem hiding this comment.
I tested this with "http://localhost:8983" and "http://127.0.0.1:8983" and it worked as expected.... I am not a kotlin expert, so hesitant to approve and merge. Hopefully @malliaridis can approve!
|
Tests are running. |
|
@epugh Thanks a lot for testing:) Despite of having used Kotlin in the past, I have no experience with Kotlin's Multiplatform, so definitely a must to get it checked by someone who actually understands it. |
malliaridis
left a comment
There was a problem hiding this comment.
Thanks a lot for addressing this issue, it was bothering me a lot during development, but I didn't took the time to address it myself.
Your implementation looks clean and makes proper use of the expect / actual pattern of Kotlin Multiplatform. I have added a note about the reason for the previous implementation with the placeholder, so that you have a bit more context.
I have only one concern about the platform-specific file namings, that normally are suffixed with [File].[platform].kt, for example PlatformUtils.wasmJs.kt. I have faced some build issues when the implementation was merged together for individual targets. You don't have to change anything yet. If that happens in a release, we will address it separately (I am also investigating the cases when this issue happens).
@renatoh Feel free to add me for any kotlin-specific changes as reviewer, and if you are interested I can provide a few more details about Kotlin stuff. I would also be very grateful if you have the time to review some of my changes or give the implementation of new features a shot. :)
|
@renatoh I leave the decision of how to resolve my feedback topics to you, and once resolved we can merge this PR. :) |
…rather than a hard coded, and potentially wrong, placeHolder URL
Are there also ticket which require mainly backend work? If so, I am happy to give it the shot. I am not good with front-end stuff and have never enjoyed it. |
Sure, there is plenty of things to do in backend. Specifically for the new UI / consumers, there are some API related changes that would be useful, specficically around configurations, but also other areas. I have only created SOLR-18191 so far, which is about validation of API requests. Feel free to take a look. I plan to create a few more API related issues once I reach that state in the frontend. :) |
(cherry picked from commit fb7377c)
At different places, e.g. on the placeholder of the welcome screen, 127.0.0.1:8983 was hard coded, that causes issues when Solr was opened using a different URL, e.g. localhost or any other URL.
Now the URL is taken from window.location.origin. Doing so the todo in Main.kt could be removed.
Additionally the right URL is now pre-populated to solr_url_input on the welcome screen, rather than having hard coded 127.0.0.1:8983 as place-holder in it.