Remove dropdowns from Mimir Admin's landing (#1549)

* Remove dropdowns from Mimir Admin's landing I didn't like the dropdowns, and Peter gave the same feedback. The issue about dropdowns is that you have to click stuff to find information that should be in the first level. This is also more accessible. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Update CHANGELOG.md Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Fix typo in the type name Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Replace panel boxes with simple horizontal lines Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com> * Make group titles be <h2> tags for accessibility Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
grafana · Mar 25, 2022 · e1adf8a · e1adf8a
1 parent afb751d
commit e1adf8a
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 67 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,7 +6,7 @@
 * [FEATURE] Ruler: Allow setting `evaluation_delay` for each rule group via rules group configuration file. #1474
 * [FEATURE] Distributor: Added the ability to forward specifics metrics to alternative remote_write API endpoints. #1052
 * [ENHANCEMENT] Ruler: Add more detailed query information to ruler query stats logging. #1411
-* [ENHANCEMENT] Admin: Admin API now has some styling. #1482
+* [ENHANCEMENT] Admin: Admin API now has some styling. #1482 #1549
 * [BUGFIX] Query-frontend: do not shard queries with a subquery unless the subquery is inside a shardable aggregation function call. #1542
 
 ### Mixin

diff --git a/pkg/api/api.go b/pkg/api/api.go
@@ -188,7 +188,7 @@ func (a *API) newRoute(path string, handler http.Handler, isPrefix, auth, gzip b
 func (a *API) RegisterAlertmanager(am *alertmanager.MultitenantAlertmanager, apiEnabled bool, buildInfoHandler http.Handler) {
 	alertmanagerpb.RegisterAlertmanagerServer(a.server.GRPC, am)
 
-	a.indexPage.AddDropdown(defaultWeight, "Alertmanager", []IndexPageDropdownElement{
+	a.indexPage.AddLinks(defaultWeight, "Alertmanager", []IndexPageLink{
 		{Desc: "Status", Path: "/multitenant_alertmanager/status"},
 		{Desc: "Ring status", Path: "/multitenant_alertmanager/ring"},
 	})
@@ -214,7 +214,7 @@ func (a *API) RegisterAlertmanager(am *alertmanager.MultitenantAlertmanager, api
 
 // RegisterAPI registers the standard endpoints associated with a running Mimir.
 func (a *API) RegisterAPI(httpPathPrefix string, actualCfg interface{}, defaultCfg interface{}, buildInfoHandler http.Handler) {
-	a.indexPage.AddDropdown(configWeight, "Current config", []IndexPageDropdownElement{
+	a.indexPage.AddLinks(configWeight, "Current config", []IndexPageLink{
 		{Desc: "Including the default values", Path: "/config"},
 		{Desc: "Only values that differ from the defaults", Path: "/config?mode=diff"},
 	})
@@ -228,7 +228,7 @@ func (a *API) RegisterAPI(httpPathPrefix string, actualCfg interface{}, defaultC
 
 // RegisterRuntimeConfig registers the endpoints associates with the runtime configuration
 func (a *API) RegisterRuntimeConfig(runtimeConfigHandler http.HandlerFunc) {
-	a.indexPage.AddDropdown(runtimeConfigWeight, "Current runtime config", []IndexPageDropdownElement{
+	a.indexPage.AddLinks(runtimeConfigWeight, "Current runtime config", []IndexPageLink{
 		{Desc: "Entire runtime config (including overrides)", Path: "/runtime_config"},
 		{Desc: "Only values that differ from the defaults", Path: "/runtime_config?mode=diff"},
 	})
@@ -242,7 +242,7 @@ func (a *API) RegisterDistributor(d *distributor.Distributor, pushConfig distrib
 
 	a.RegisterRoute("/api/v1/push", push.Handler(pushConfig.MaxRecvMsgSize, a.sourceIPs, a.cfg.SkipLabelNameValidationHeader, a.cfg.wrapDistributorPush(d)), true, false, "POST")
 
-	a.indexPage.AddDropdown(defaultWeight, "Distributor", []IndexPageDropdownElement{
+	a.indexPage.AddLinks(defaultWeight, "Distributor", []IndexPageLink{
 		{Desc: "Ring status", Path: "/distributor/ring"},
 		{Desc: "Usage statistics", Path: "/distributor/all_user_stats"},
 		{Desc: "HA tracker status", Path: "/distributor/ha_tracker"},
@@ -266,7 +266,7 @@ type Ingester interface {
 func (a *API) RegisterIngester(i Ingester, pushConfig distributor.Config) {
 	client.RegisterIngesterServer(a.server.GRPC, i)
 
-	a.indexPage.AddDropdown(dangerousWeight, "Dangerous", []IndexPageDropdownElement{
+	a.indexPage.AddLinks(dangerousWeight, "Dangerous", []IndexPageLink{
 		{Dangerous: true, Desc: "Trigger a flush of data from ingester to storage", Path: "/ingester/flush"},
 		{Dangerous: true, Desc: "Trigger ingester shutdown", Path: "/ingester/shutdown"},
 	})
@@ -283,7 +283,7 @@ func (a *API) RegisterTenantDeletion(api *purger.TenantDeletionAPI) {
 
 // RegisterRuler registers routes associated with the Ruler service.
 func (a *API) RegisterRuler(r *ruler.Ruler) {
-	a.indexPage.AddDropdown(defaultWeight, "Ruler", []IndexPageDropdownElement{
+	a.indexPage.AddLinks(defaultWeight, "Ruler", []IndexPageLink{
 		{Desc: "Ring status", Path: "/ruler/ring"},
 	})
 	a.RegisterRoute("/ruler/ring", r, false, true, "GET", "POST")
@@ -336,7 +336,7 @@ func (a *API) RegisterRulerAPI(r *ruler.API, configAPIEnabled bool) {
 
 // RegisterRing registers the ring UI page associated with the distributor for writes.
 func (a *API) RegisterRing(r http.Handler) {
-	a.indexPage.AddDropdown(defaultWeight, "Ingester", []IndexPageDropdownElement{
+	a.indexPage.AddLinks(defaultWeight, "Ingester", []IndexPageLink{
 		{Desc: "Ring status", Path: "/ingester/ring"},
 	})
 	a.RegisterRoute("/ingester/ring", r, false, true, "GET", "POST")
@@ -346,7 +346,7 @@ func (a *API) RegisterRing(r http.Handler) {
 func (a *API) RegisterStoreGateway(s *storegateway.StoreGateway) {
 	storegatewaypb.RegisterStoreGatewayServer(a.server.GRPC, s)
 
-	a.indexPage.AddDropdown(defaultWeight, "Store-gateway", []IndexPageDropdownElement{
+	a.indexPage.AddLinks(defaultWeight, "Store-gateway", []IndexPageLink{
 		{Desc: "Ring status", Path: "/store-gateway/ring"},
 		{Desc: "Tenants & Blocks", Path: "/store-gateway/tenants"},
 	})
@@ -357,7 +357,7 @@ func (a *API) RegisterStoreGateway(s *storegateway.StoreGateway) {
 
 // RegisterCompactor registers the ring UI page associated with the compactor.
 func (a *API) RegisterCompactor(c *compactor.MultitenantCompactor) {
-	a.indexPage.AddDropdown(defaultWeight, "Compactor", []IndexPageDropdownElement{
+	a.indexPage.AddLinks(defaultWeight, "Compactor", []IndexPageLink{
 		{Desc: "Ring status", Path: "/compactor/ring"},
 	})
 	a.RegisterRoute("/compactor/ring", http.HandlerFunc(c.RingHandler), false, true, "GET", "POST")
@@ -417,11 +417,15 @@ func (a *API) RegisterQueryScheduler(f *scheduler.Scheduler) {
 // TODO: Refactor this code to be accomplished using the services.ServiceManager
 // or a future module manager #2291
 func (a *API) RegisterServiceMapHandler(handler http.Handler) {
-	a.indexPage.AddLink(serviceStatusWeight, "Service status", "/services")
+	a.indexPage.AddLinks(serviceStatusWeight, "Overview", []IndexPageLink{
+		{Desc: "Service status", Path: "/services"},
+	})
 	a.RegisterRoute("/services", handler, false, true, "GET")
 }
 
 func (a *API) RegisterMemberlistKV(handler http.Handler) {
-	a.indexPage.AddLink(memberlistWeight, "Memberlist status", "/memberlist")
+	a.indexPage.AddLinks(memberlistWeight, "Memberlist", []IndexPageLink{
+		{Desc: "Status", Path: "/memberlist"},
+	})
 	a.RegisterRoute("/memberlist", handler, false, true, "GET")
 }
diff --git a/pkg/api/handlers.go b/pkg/api/handlers.go
@@ -43,22 +43,23 @@ func newIndexPageContent() *IndexPageContent {
 type IndexPageContent struct {
 	mu sync.Mutex
 
-	elements []IndexPageNavElement
+	elements []IndexPageLinkGroup
 }
 
-type IndexPageNavElement struct {
-	weight   int
-	Desc     string
-	Path     string
-	Dropdown []IndexPageDropdownElement
+type IndexPageLinkGroup struct {
+	weight int
+	Desc   string
+	Path   string
+	Links  []IndexPageLink
 }
 
-type IndexPageDropdownElement struct {
+type IndexPageLink struct {
 	Desc      string
 	Path      string
 	Dangerous bool
 }
 
+// List of weights to order link groups in the same order as weights are ordered here.
 const (
 	serviceStatusWeight = iota
 	configWeight
@@ -68,23 +69,16 @@ const (
 	dangerousWeight
 )
 
-func (pc *IndexPageContent) AddLink(weight int, desc, path string) {
+func (pc *IndexPageContent) AddLinks(weight int, groupDesc string, links []IndexPageLink) {
 	pc.mu.Lock()
 	defer pc.mu.Unlock()
 
-	pc.elements = append(pc.elements, IndexPageNavElement{weight: weight, Desc: desc, Path: path})
+	pc.elements = append(pc.elements, IndexPageLinkGroup{weight: weight, Desc: groupDesc, Links: links})
 }
 
-func (pc *IndexPageContent) AddDropdown(weight int, desc string, elements []IndexPageDropdownElement) {
+func (pc *IndexPageContent) GetContent() []IndexPageLinkGroup {
 	pc.mu.Lock()
-	defer pc.mu.Unlock()
-
-	pc.elements = append(pc.elements, IndexPageNavElement{weight: weight, Desc: desc, Dropdown: elements})
-}
-
-func (pc *IndexPageContent) GetContent() []IndexPageNavElement {
-	pc.mu.Lock()
-	els := append([]IndexPageNavElement(nil), pc.elements...)
+	els := append([]IndexPageLinkGroup(nil), pc.elements...)
 	pc.mu.Unlock()
 
 	sort.Slice(els, func(i, j int) bool {

diff --git a/pkg/api/handlers_test.go b/pkg/api/handlers_test.go
@@ -18,17 +18,12 @@ import (
 
 func TestIndexHandlerPrefix(t *testing.T) {
 	c := newIndexPageContent()
-	c.AddLink(defaultWeight, "Ingester Ring", "/ingester/ring")
-	c.AddDropdown(defaultWeight, "Store Gateway", []IndexPageDropdownElement{{Desc: "Ring status", Path: "/store-gateway/ring"}})
+	c.AddLinks(defaultWeight, "Store Gateway", []IndexPageLink{{Desc: "Ring status", Path: "/store-gateway/ring"}})
 
 	for _, tc := range []struct {
 		prefix    string
 		toBeFound string
 	}{
-		{prefix: "", toBeFound: "<a href=\"/ingester/ring\">"},
-		{prefix: "/test", toBeFound: "<a href=\"/test/ingester/ring\">"},
-		// All the extra slashed are cleaned up in the result.
-		{prefix: "///test///", toBeFound: "<a href=\"/test/ingester/ring\">"},
 		{prefix: "", toBeFound: "<a href=\"/store-gateway/ring\">"},
 		{prefix: "/test", toBeFound: "<a href=\"/test/store-gateway/ring\">"},
 		// All the extra slashed are cleaned up in the result.
@@ -48,9 +43,8 @@ func TestIndexHandlerPrefix(t *testing.T) {
 
 func TestIndexPageContent(t *testing.T) {
 	c := newIndexPageContent()
-	c.AddLink(defaultWeight, "Ingester Ring", "/ingester/ring")
-	c.AddDropdown(defaultWeight, "Store Gateway", []IndexPageDropdownElement{
-		{Desc: "Ring status", Path: "/store-gateway/ring"},
+	c.AddLinks(defaultWeight, "Some group", []IndexPageLink{
+		{Desc: "Some link", Path: "/store-gateway/ring"},
 		{Dangerous: true, Desc: "Boom!", Path: "/store-gateway/boom"},
 	})
 
@@ -62,10 +56,11 @@ func TestIndexPageContent(t *testing.T) {
 	h.ServeHTTP(resp, req)
 
 	require.Equal(t, 200, resp.Code)
-	require.True(t, strings.Contains(resp.Body.String(), "Ingester Ring"))
-	require.True(t, strings.Contains(resp.Body.String(), "/ingester/ring"))
+	require.True(t, strings.Contains(resp.Body.String(), "Some group"))
+	require.True(t, strings.Contains(resp.Body.String(), "Some link"))
+	require.True(t, strings.Contains(resp.Body.String(), "Dangerous"))
+	require.True(t, strings.Contains(resp.Body.String(), "Boom!"))
 	require.True(t, strings.Contains(resp.Body.String(), "Dangerous"))
-	require.True(t, strings.Contains(resp.Body.String(), "Ring status"))
 	require.True(t, strings.Contains(resp.Body.String(), "/store-gateway/ring"))
 	require.True(t, strings.Contains(resp.Body.String(), "/store-gateway/boom"))
 	require.False(t, strings.Contains(resp.Body.String(), "/compactor/ring"))

diff --git a/pkg/api/index.gohtml b/pkg/api/index.gohtml
@@ -14,33 +14,26 @@
 <body>
 <div class="container">
     <div class="page-header">
-        <h1>Grafana Mimir Admin<img class="mimir-brand pull-right" src="{{ AddPathPrefix "/static/mimir-icon.png" }}"></h1>
+        <h1>
+            Grafana Mimir Admin
+            <img alt="Mimir logo" class="mimir-brand pull-right" src="{{ AddPathPrefix "/static/mimir-icon.png" }}">
+        </h1>
     </div>
-    <ul class="nav nav-pills nav-stacked">
-        {{ range . }}
-            {{- /*gotype: github.com/grafana/mimir/pkg/api.IndexPageNavElement */ -}}
-            {{ if .Dropdown }}
-                <li role="presentation" class="dropdown">
-                    <a class="dropdown-toggle" data-toggle="dropdown" href="#" role="button" aria-haspopup="true"
-                       aria-expanded="false">
-                        {{ .Desc }} <span class="caret"></span>
-                    </a>
-                    <ul class="dropdown-menu">
-                        {{ range .Dropdown }}
-                            <li>
-                                <a href="{{ AddPathPrefix .Path }}">
-                                    {{ if .Dangerous }}<span class="label label-danger">Dangerous</span>{{ end }}
-                                    {{ .Desc }}
-                                </a>
-                            </li>
-                        {{ end }}
-                    </ul>
-                </li>
-            {{ else }}
-                <li role="presentation"><a href="{{ AddPathPrefix .Path }}">{{ .Desc }}</a></li>
-            {{ end }}
+    <dl class="dl-horizontal">
+    {{ range $i, $ := . }}
+        {{- /*gotype: github.com/grafana/mimir/pkg/api.IndexPageLinkGroup */ -}}
+        {{ if $i }}
+            <hr>
         {{ end }}
-    </ul>
+            <dt><h2>{{ .Desc }}</h2></dt>
+            {{ range .Links }}
+                <dd>
+                    <a href="{{ AddPathPrefix .Path }}">{{ .Desc }}</a>
+                    {{ if .Dangerous }}<span class="label label-danger">Dangerous</span>{{ end }}
+                </dd>
+            {{ end }}
+    {{ end }}
+    </dl>
 </div>
 
 <script src="{{ AddPathPrefix "/static/jquery-1.12.4.min.js" }}"></script>

diff --git a/pkg/api/static/mimir-styles.css b/pkg/api/static/mimir-styles.css
@@ -1,4 +1,10 @@
 
 .page-header .mimir-brand {
     height: 34px;
+}
+
+.dl-horizontal dt h2 {
+    display: inline;
+    font-size: 14px; /* Same as default for the body */
+    font-weight: 700; /* Same as dt */
 }