Merge pull request #1612 from rust-lang/compare-page-ui

Update compare page UI

Author: Kobzol

site/frontend/package.json

@@ -3,7 +3,7 @@
   "version": "1.0.0",
   "description": "",
   "scripts": {
-    "watch": "parcel watch",
+    "watch": "parcel watch --no-hmr",
     "build": "parcel build",
     "fmt": "prettier --write src",
     "check": "tsc -p tsconfig.json --noEmit && prettier --check src"

site/frontend/src/pages/compare/benchmarks/test-cases-table.vue

@@ -117,29 +117,41 @@ function prettifyRawNumber(number: number): string {
             </td>
             <td>{{ testCase.scenario }}</td>
             <td>
-              <a
-                v-bind:href="
-                  detailedQueryPercentLink(commitB, commitA, testCase)
-                "
-              >
-                <span v-bind:class="percentClass(testCase.percent)">
-                  {{ testCase.percent.toFixed(2) }}%
-                </span>
-              </a>
+              <div class="numeric-aligned">
+                <div>
+                  <a
+                    v-bind:href="
+                      detailedQueryPercentLink(commitB, commitA, testCase)
+                    "
+                  >
+                    <span v-bind:class="percentClass(testCase.percent)">
+                      {{ testCase.percent.toFixed(2) }}%
+                    </span>
+                  </a>
+                </div>
+              </div>
             </td>
             <td>
-              {{
-                testCase.significanceThreshold
-                  ? testCase.significanceThreshold.toFixed(2) + "%"
-                  : "-"
-              }}
+              <div class="numeric-aligned">
+                <div>
+                  {{
+                    testCase.significanceThreshold
+                      ? testCase.significanceThreshold.toFixed(2) + "%"
+                      : "-"
+                  }}
+                </div>
+              </div>
             </td>
             <td>
-              {{
-                testCase.significanceFactor
-                  ? testCase.significanceFactor.toFixed(2) + "x"
-                  : "-"
-              }}
+              <div class="numeric-aligned">
+                <div>
+                  {{
+                    testCase.significanceFactor
+                      ? testCase.significanceFactor.toFixed(2) + "x"
+                      : "-"
+                  }}
+                </div>
+              </div>
             </td>
             <td v-if="showRawData" class="numeric">
               <a v-bind:href="detailedQueryRawDataLink(commitA, testCase)">
@@ -189,17 +201,27 @@ function prettifyRawNumber(number: number): string {
 
 .benches th {
   text-align: center;
-  width: 25%;
   min-width: 50px;
 }
 
 .benches td {
   text-align: center;
   width: 25%;
-}
 
-.benches td.numeric {
-  text-align: right;
+  & > .numeric-aligned {
+    display: flex;
+    flex-direction: column;
+    align-items: center;
+    text-align: right;
+
+    & > div {
+      width: 70px;
+    }
+  }
+
+  &.numeric {
+    text-align: right;
+  }
 }
 
 .bench-table {

site/frontend/src/pages/compare/bootstrap-table.vue

@@ -1,6 +1,6 @@
 <script setup lang="ts">
 import {CompareResponse} from "./types";
-import {percentClass} from "./shared";
+import {diffClass, percentClass} from "./shared";
 
 const props = defineProps<{data: CompareResponse}>();
 
@@ -44,16 +44,6 @@ const bootstraps = Object.entries(props.data.a.bootstrap)
       return a.name.localeCompare(b.name);
     }
   });
-
-function diffClass(diff: number): string {
-  let klass = "";
-  if (diff > 1) {
-    klass = "positive";
-  } else if (diff < -1) {
-    klass = "negative";
-  }
-  return klass;
-}
 </script>
 
 <template>

site/frontend/src/pages/compare/header/filters.vue

@@ -33,7 +33,7 @@ watch(
 
 <template>
   <fieldset class="collapsible-section">
-    <Toggle :defaultOpened="true">
+    <Toggle :defaultOpened="false">
       <template #label>Filters</template>
       <template #content>
         <div>

site/frontend/src/pages/compare/header/header.vue

@@ -61,7 +61,7 @@ const after = computed(() =>
 </script>
 
 <template>
-  <h2>
+  <h2 style="text-align: center">
     Comparing <span id="stat-header">{{ selector.stat }}</span> between
     <span id="before">{{ before }}</span> and
     <span id="after">{{ after }}</span>

site/frontend/src/pages/compare/header/quick-links.vue

@@ -39,7 +39,7 @@ const metrics = [
 
 <template>
   <div class="quick-links">
-    <div>Quick links:</div>
+    <div class="label">Quick links:</div>
     <div
       v-for="metric in metrics"
       :class="{active: props.stat === metric.stat}"
@@ -64,4 +64,12 @@ const metrics = [
 .quick-links .active {
   font-weight: bold;
 }
+
+.quick-links .label {
+  display: none;
+
+  @media (min-width: 600px) {
+    display: block;
+  }
+}
 </style>

site/frontend/src/pages/compare/page.vue

@@ -2,7 +2,7 @@
 import {loadBenchmarkInfo} from "../../api";
 import AsOf from "../../components/as-of.vue";
 import {
-  createUrlFromParams,
+  changeUrl,
   createUrlWithAppendedParams,
   getUrlParams,
   navigateToUrlParams,
@@ -11,7 +11,7 @@ import {computed, Ref, ref} from "vue";
 import {withLoading} from "../../utils/loading";
 import {postMsgpack} from "../../utils/requests";
 import {COMPARE_DATA_URL} from "../../urls";
-import {CompareResponse, CompareSelector, DataFilter} from "./types";
+import {CompareResponse, CompareSelector, DataFilter, Tab} from "./types";
 import BootstrapTable from "./bootstrap-table.vue";
 import Header from "./header/header.vue";
 import DataSelector, {SelectionParams} from "./header/data-selector.vue";
@@ -23,10 +23,12 @@ import {
   computeSummary,
   computeTestCasesWithNonRelevant,
   filterNonRelevant,
+  SummaryGroup,
 } from "./data";
 import OverallTable from "./summary/overall-table.vue";
 import Aggregations from "./summary/aggregations.vue";
 import Benchmarks from "./benchmarks/benchmarks.vue";
+import Tabs from "./tabs.vue";
 
 function loadSelectorFromUrl(urlParams: Dict<string>): CompareSelector {
   const start = urlParams["start"] ?? "";
@@ -39,6 +41,21 @@ function loadSelectorFromUrl(urlParams: Dict<string>): CompareSelector {
   };
 }
 
+function loadTabFromUrl(urlParams: Dict<string>): Tab | null {
+  const tab = urlParams["tab"] ?? "";
+  if (tab == Tab.CompileTime) {
+    return Tab.CompileTime;
+  } else if (tab == Tab.Bootstrap) {
+    return Tab.Bootstrap;
+  }
+  return null;
+}
+
+function storeTabToUrl(urlParams: Dict<string>, tab: Tab) {
+  urlParams["tab"] = tab as string;
+  changeUrl(urlParams);
+}
+
 function loadFilterFromUrl(
   urlParams: Dict<string>,
   defaultFilter: DataFilter
@@ -139,10 +156,7 @@ function storeFilterToUrl(
     defaultFilter.category.secondary
   );
 
-  // Change URL without creating a history entry
-  if (history.replaceState) {
-    history.replaceState({}, null, createUrlFromParams(urlParams).toString());
-  }
+  changeUrl(urlParams);
 }
 
 async function loadCompareData(
@@ -158,6 +172,16 @@ async function loadCompareData(
     return await postMsgpack<CompareResponse>(COMPARE_DATA_URL, params);
   });
   data.value = response;
+  totalSummary.value = computeSummary(
+    filterNonRelevant(
+      defaultFilter,
+      computeTestCasesWithNonRelevant(
+        defaultFilter,
+        data.value,
+        benchmarkMap.value
+      )
+    )
+  );
 }
 
 function updateSelection(params: SelectionParams) {
@@ -222,13 +246,26 @@ const testCases = computed(() =>
 );
 const filteredSummary = computed(() => computeSummary(testCases.value));
 
+// Include all relevant changes in the compile-time tab summary
+// We do not wrap it in computed, because it should be loaded only once, after
+// the data is downloaded.
+const totalSummary: Ref<SummaryGroup | null> = ref(null);
+
 const loading = ref(false);
 
 const quickLinksKey = ref(0);
 const info = await loadBenchmarkInfo();
 const selector = loadSelectorFromUrl(urlParams);
 const filter = ref(loadFilterFromUrl(urlParams, defaultFilter));
 
+const initialTab: Tab = loadTabFromUrl(urlParams) ?? Tab.CompileTime;
+const tab: Ref<Tab> = ref(initialTab);
+
+function changeTab(newTab: Tab) {
+  tab.value = newTab;
+  storeTabToUrl(getUrlParams(), newTab);
+}
+
 const data: Ref<CompareResponse | null> = ref(null);
 loadCompareData(selector, loading);
 </script>
@@ -247,23 +284,31 @@ loadCompareData(selector, loading);
       <p>Loading ...</p>
     </div>
     <div v-if="data !== null">
-      <QuickLinks :stat="selector.stat" :key="quickLinksKey" />
-      <Filters
-        :defaultFilter="defaultFilter"
-        :initialFilter="filter"
-        @change="updateFilter"
-        @export="exportData"
-      />
-      <OverallTable :summary="filteredSummary" />
-      <Aggregations :cases="testCases" />
-      <Benchmarks
+      <Tabs
+        @change-tab="changeTab"
         :data="data"
-        :test-cases="testCases"
-        :all-test-cases="allTestCases"
-        :filter="filter"
-        :stat="selector.stat"
-      ></Benchmarks>
-      <BootstrapTable :data="data" />
+        :initial-tab="initialTab"
+        :compile-time-summary="totalSummary"
+      />
+      <template v-if="tab === Tab.CompileTime">
+        <QuickLinks :stat="selector.stat" :key="quickLinksKey" />
+        <Filters
+          :defaultFilter="defaultFilter"
+          :initialFilter="filter"
+          @change="updateFilter"
+          @export="exportData"
+        />
+        <OverallTable :summary="filteredSummary" />
+        <Aggregations :cases="testCases" />
+        <Benchmarks
+          :data="data"
+          :test-cases="testCases"
+          :all-test-cases="allTestCases"
+          :filter="filter"
+          :stat="selector.stat"
+        ></Benchmarks>
+      </template>
+      <BootstrapTable v-if="tab === Tab.Bootstrap" :data="data" />
     </div>
   </div>
   <br />

site/frontend/src/pages/compare/shared.ts

@@ -27,3 +27,10 @@ export function percentClass(pct: number): string {
   }
   return klass;
 }
+
+export function diffClass(diff: number): string {
+  if (diff >= 0) {
+    return "positive";
+  }
+  return "negative";
+}

site/frontend/src/pages/compare/summary/percent-value.vue

@@ -1,6 +1,6 @@
 <script setup lang="ts">
 import {computed} from "vue";
-import {signIfPositive} from "../shared";
+import {percentClass, signIfPositive} from "../shared";
 
 const props = withDefaults(
   defineProps<{
@@ -23,5 +23,7 @@ const padSpaces = computed((): string => {
 </script>
 
 <template>
-  <span><span v-html="padSpaces" />{{ formattedValue }}%</span>
+  <span :class="percentClass(props.value)"
+    ><span v-html="padSpaces" />{{ formattedValue }}%</span
+  >
 </template>

site/frontend/src/pages/compare/summary/summary-table.vue

@@ -39,7 +39,7 @@ const summary = computed(() => props.summary);
       </tr>
     </thead>
     <tbody>
-      <tr class="positive">
+      <tr>
         <td title="Regressions" v-if="withLegend">❌</td>
         <template v-if="summary.regressions.count !== 0">
           <td>
@@ -48,7 +48,7 @@ const summary = computed(() => props.summary);
           <td>
             <SummaryPercentValue :value="summary.regressions.average" />
           </td>
-          <td>
+          <td class="positive">
             <SummaryCount
               :cases="summary.regressions.count"
               :benchmarks="summary.regressions.benchmarks"
@@ -59,7 +59,7 @@ const summary = computed(() => props.summary);
           <td colspan="3" style="text-align: center">No regressions</td>
         </template>
       </tr>
-      <tr class="negative">
+      <tr>
         <td title="Improvements" v-if="withLegend">✅</td>
         <template v-if="summary.improvements.count !== 0">
           <td>
@@ -68,7 +68,7 @@ const summary = computed(() => props.summary);
           <td>
             <SummaryPercentValue :value="summary.improvements.average" />
           </td>
-          <td>
+          <td class="negative">
             <SummaryCount
               :cases="summary.improvements.count"
               :benchmarks="summary.improvements.benchmarks"