-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
ui: modify content path #5950
ui: modify content path #5950
Conversation
@s-christoff Looks like you need to run |
Great set of changes - looking forward to having context path finally! 🎉 |
…y at runtime (#5934) * ui: Only inject {{.ContentPath}} if we are makeing a prod build... ...otherwise we just use the current rootURL This gets injected into a <base /> node which solves the assets path problem but not the ember problem * ui: Pull out the <base href=""> value and inject it into ember env See previous commit: The <base href=""> value is 'sometimes' injected from go at index serve time. We pass this value down to ember by overwriting the ember config that is injected via a <meta> tag. This has to be done before ember bootup. Sometimes (during testing and development, basically not production) this is injected with the already existing value, in which case this essentially changes nothing. The code here is slightly abstracted away from our specific usage to make it easier for anyone else to use, and also make sure we can cope with using this same method to pass variables down from the CLI through to ember in the future.
Unfortuantely we can't seem to be able to use <base> and rootURL together as URL paths will get doubled up (`ui/ui/`). This moves all the things that we need to interpolate with .ContentPath to the `startup` javascript so we can conditionally print out `{{.ContentPath}}` in lots of places (now we can't use base)
…5945) ...and potentially more environments Testing has more additional things in a separate index.html in `tests/` This make the entire thing a little saner and uses just javascriopt template literals instead of a pseudo handbrake synatx for our templating of these files. Intead of just templating the entire file this way, we still only template `{{content-for 'head'}}` and `{{content-for 'body'}}` in this way to ensure we support other plugins/addons
* build: Loosen up the regex for retrieving the CONSUL_VERSION 1. Previously the `sed` replacement was searching for the CONSUL_VERSION comment at the start of a line, it no longer does this to allow for indentation. 2. Both `grep` and `sed` where looking for the omment at the end of the line. We've removed this restriction here. We don't need to remove it right now, but if we ever put the comment followed by something here the searching would break. 3. Added `xargs` for trimming the resulting version string. We aren't using this already in the rest of the scripts, but we are pretty sure this is available on most systems. * ui: Fix erroneous variable, and also force an ember cache clean on build 1. We referenced a variable incorrectly here, this fixes that. 2. We also made sure that every `make` target clears ember's `tmp` cache to ensure that its not using any caches that have since been edited everytime we call a `make` target.
Co-Authored-By: R.B. Boyer <[email protected]>
ed98ef1
to
226bda2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Co-Authored-By: Hans Hasselberg <[email protected]>
Co-Authored-By: kaitlincarter-hc <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to premptively approve but there is one minor change to remove the no longer used ContentPath
field from the redirectFS
struct.
agent/http.go
Outdated
|
||
type redirectFS struct { | ||
fs http.FileSystem | ||
fs http.FileSystem | ||
ContentPath string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can get rid of the ContentPath here
Allows user to replace
/ui/
with their own path as long as it contains alphanumeric, -, _, or /.Fixes issue #1930 , however you cannot specify
v1
as that conflicts with the API endpoint.At this time, you cannot specify a slash to strip the ending prefix or to be used with a subdomain.