UI V2 #4086
UI V2 #4086
Conversation
Add some todo's in to remind me to think consider this further at a later date. For example, is normalizePayload to be a hook or an overridable method
Feature/normalize payload
Move away from using `reopen` to using Mixins
There was a problem hiding this comment.
Go bits reviewed. It is all good but suggested some simplifications/style tweaks etc.
Great job.
Incidentally, I'm in two minds about whether it would be better to squash merge this or rebase your branch to squash those commits - over 250 commits is a lot to merge. In my mind it's easier to grok the master history if major features like this are added in a single squashed commit during merge but that does loose history from your branch and not every one on the team agrees with me ;). No big deal either way but if you're not sure we can bikeshed it in Slack.
agent/http.go
Outdated
| @@ -41,6 +42,18 @@ type HTTPServer struct { | |||
| proto string | |||
| } | |||
|
|
|||
| type RedirectFS struct { | |||
There was a problem hiding this comment.
Nitpick: public types and functions should really have a comment. We don't always do this in Consul currently but IMO it's a convention that's worth keeping. Example for this would be:
// RedirectFS is an http.FileSystem implementation that ... We use it because...
// Open implements http.FileSystem
agent/http.go
Outdated
| func (fs *RedirectFS) Open(name string) (http.File, error) { | ||
| file, err := fs.fs.Open(name) | ||
| if err != nil { | ||
| file, err = fs.fs.Open("/index.html") |
There was a problem hiding this comment.
Was this copied from another project? I know we do similar things in other places for example https://github.com/hashicorp/terraform-registry/blob/master/helper/fallback-fs/fallback.go.
There was a problem hiding this comment.
Nope, I just implemented it myself. I didn't think to look for this particular thing in another project.
agent/http.go
Outdated
| new_ui = true | ||
| default: | ||
| new_ui = false | ||
| } |
There was a problem hiding this comment.
Can just use:
new_ui, err := strconv.ParseBool(os.Getenv("CONSUL_UI_BETA"))
if err != nil {
// Invalid/empty value is treated as false
new_ui = false
}There was a problem hiding this comment.
Yeah, I can do that for simplicity. IMO ParseBool should also handle y,Y,Yes etc as true values but it doesn't. Many env variables I have used in other products and had in my own in the past like yes/no values for things. I was trying to keep it intuitive. Then again people using Consul are probably used to how we handle things anyways.
agent/http.go
Outdated
| default: | ||
| new_ui = false | ||
| } | ||
| var uifs http.FileSystem = nil; |
There was a problem hiding this comment.
= nil is redundant as it's the zero value for an interface anyway.
This PR includes a redesigned/rebuilt user interface using the Ember v2 framework. The primary aim is to provide feature parity with the current user interface along with providing a solid foundation for moving forwards adding new features and improvements to the user interface.
Further addition in this PR include changes to enable 404 redirection as is required for a modern SPA, plus the usage of a
CONSUL_UI_BETAenvironment variable to enable this new user interface.Setting the
CONSUL_UI_BETAenvironment variable to a 'truthy' value and specifying the-uicommand line flag will enable the new user interface.