From f79799fef7304b28c9275ce0da9fac3789060cf1 Mon Sep 17 00:00:00 2001
From: Milutin Kristofic <milutin@google.com>
Date: Thu, 24 Aug 2023 11:56:11 +0200
Subject: Improve Submit requirements for multiple labels

- Fix bug where more labels introduce more horizontal space caused
by .separator class
- Add labels when there is more than 1 label voted on in 1 submit
requirement

Release-Notes: skip
Google-Bug-Id: b/296450377
Change-Id: I10d1c0668396c58aea365ea1c2a9e37f756f4c0d
---
 .../gr-submit-requirements.ts                      | 66 ++++++++++++++--------
 .../gr-submit-requirements_test.ts                 | 51 ++++++++++++-----
 2 files changed, 80 insertions(+), 37 deletions(-)

diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
index 640b9d025d..29a22a36b9 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
@@ -42,7 +42,6 @@ import {Tab} from '../../../constants/constants';
 import {submitRequirementsStyles} from '../../../styles/gr-submit-requirements-styles';
 import {resolve} from '../../../models/dependency';
 import {checksModelToken} from '../../../models/checks/checks-model';
-import {join} from 'lit/directives/join.js';
 import {map} from 'lit/directives/map.js';
 
 /**
@@ -111,13 +110,14 @@ export class GrSubmitRequirements extends LitElement {
           white-space: nowrap;
           vertical-align: top;
         }
-        .votes-cell {
+        .votes {
           display: flex;
-          flex-flow: wrap;
+          flex-flow: column;
+          row-gap: var(--spacing-s);
         }
-        .votes-cell .separator {
-          width: 100%;
-          margin-top: var(--spacing-s);
+        .votes-line {
+          display: flex;
+          flex-flow: wrap;
         }
         gr-vote-chip {
           margin-right: var(--spacing-s);
@@ -280,16 +280,21 @@ export class GrSubmitRequirements extends LitElement {
       hasVotes(allLabels[label])
     );
 
-    return html`${join(
-      map(
+    return html`<div class="votes">
+      ${map(
         associatedLabelsWithVotes,
         label =>
-          html`${this.renderLabelVote(label, allLabels)}
-          ${this.renderOverrideLabels(requirement, label)}`
-      ),
-      html`<span class="separator"></span>`
-    )}
-    ${this.renderChecks(requirement)}`;
+          html`<div class="votes-line">
+            ${this.renderLabelVote(label, allLabels)}
+            ${this.renderOverrideLabels(
+              requirement,
+              label,
+              associatedLabelsWithVotes.length > 1
+            )}
+            ${this.renderChecks(requirement, label)}
+          </div>`
+      )}
+    </div> `;
   }
 
   renderLabelVote(label: string, labels: LabelNameToInfoMap) {
@@ -317,12 +322,17 @@ export class GrSubmitRequirements extends LitElement {
 
   renderOverrideLabels(
     requirement: SubmitRequirementResultInfo,
-    forLabel: string
+    forLabel: string,
+    showForAllLabel: boolean
   ) {
-    if (requirement.status !== SubmitRequirementStatus.OVERRIDDEN) return;
+    if (
+      !showForAllLabel &&
+      requirement.status !== SubmitRequirementStatus.OVERRIDDEN
+    )
+      return;
     const requirementLabels = extractAssociatedLabels(
       requirement,
-      'onlyOverride'
+      showForAllLabel ? 'all' : 'onlyOverride'
     )
       .filter(label => label === forLabel)
       .filter(label => {
@@ -334,13 +344,17 @@ export class GrSubmitRequirements extends LitElement {
     );
   }
 
-  renderChecks(requirement: SubmitRequirementResultInfo) {
+  renderChecks(requirement: SubmitRequirementResultInfo, labelName?: string) {
     const requirementLabels = extractAssociatedLabels(requirement);
     const errorRuns = this.runs
       .filter(run => hasResultsOf(run, Category.ERROR))
-      .filter(
-        run => run.labelName && requirementLabels.includes(run.labelName)
-      );
+      .filter(run => {
+        if (labelName) {
+          return labelName === run.labelName;
+        } else {
+          return run.labelName && requirementLabels.includes(run.labelName);
+        }
+      });
     const errorRunsCount = errorRuns.reduce(
       (sum, run) => sum + getResultsOf(run, Category.ERROR).length,
       0
@@ -357,9 +371,13 @@ export class GrSubmitRequirements extends LitElement {
       .filter(
         r => r.status === RunStatus.RUNNING || r.status === RunStatus.SCHEDULED
       )
-      .filter(
-        run => run.labelName && requirementLabels.includes(run.labelName)
-      );
+      .filter(run => {
+        if (labelName) {
+          return labelName === run.labelName;
+        } else {
+          return run.labelName && requirementLabels.includes(run.labelName);
+        }
+      });
 
     const runningRunsCount = runningRuns.length;
     if (runningRunsCount > 0) {
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts
index 212394c697..a5a336cd76 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts
@@ -102,7 +102,11 @@ suite('gr-submit-requirements tests', () => {
                 >
                   <gr-endpoint-param name="change"></gr-endpoint-param>
                   <gr-endpoint-param name="requirement"></gr-endpoint-param>
-                  <gr-vote-chip></gr-vote-chip>
+                  <div class="votes">
+                    <div class="votes-line">
+                      <gr-vote-chip> </gr-vote-chip>
+                    </div>
+                  </div>
                 </gr-endpoint-decorator>
               </td>
             </tr>
@@ -125,7 +129,11 @@ suite('gr-submit-requirements tests', () => {
         votesCell?.[0],
         /* HTML */ `
           <div class="votes-cell">
-            <gr-vote-chip> </gr-vote-chip>
+            <div class="votes">
+              <div class="votes-line">
+                <gr-vote-chip> </gr-vote-chip>
+              </div>
+            </div>
           </div>
         `
       );
@@ -174,8 +182,12 @@ suite('gr-submit-requirements tests', () => {
         votesCell?.[0],
         /* HTML */ `
           <div class="votes-cell">
-            <gr-vote-chip></gr-vote-chip>
-            <gr-checks-chip></gr-checks-chip>
+            <div class="votes">
+              <div class="votes-line">
+                <gr-vote-chip> </gr-vote-chip>
+                <gr-checks-chip> </gr-checks-chip>
+              </div>
+            </div>
           </div>
         `
       );
@@ -196,8 +208,12 @@ suite('gr-submit-requirements tests', () => {
         votesCell?.[0],
         /* HTML */ `
           <div class="votes-cell">
-            <gr-vote-chip></gr-vote-chip>
-            <gr-checks-chip></gr-checks-chip>
+            <div class="votes">
+              <div class="votes-line">
+                <gr-vote-chip> </gr-vote-chip>
+                <gr-checks-chip> </gr-checks-chip>
+              </div>
+            </div>
           </div>
         `
       );
@@ -231,8 +247,12 @@ suite('gr-submit-requirements tests', () => {
       assert.dom.equal(
         votesCell?.[0],
         /* HTML */ `<div class="votes-cell">
-          <gr-vote-chip> </gr-vote-chip>
-          <span class="overrideLabel"> Override </span>
+          <div class="votes">
+            <div class="votes-line">
+              <gr-vote-chip> </gr-vote-chip>
+              <span class="overrideLabel"> Override </span>
+            </div>
+          </div>
         </div>`
       );
     });
@@ -274,11 +294,16 @@ suite('gr-submit-requirements tests', () => {
       assert.dom.equal(
         votesCell?.[0],
         /* HTML */ `<div class="votes-cell">
-          <gr-vote-chip> </gr-vote-chip>
-          <span class="overrideLabel"> Override </span>
-          <span class="separator"></span>
-          <gr-vote-chip> </gr-vote-chip>
-          <span class="overrideLabel"> Override2 </span>
+          <div class="votes">
+            <div class="votes-line">
+              <gr-vote-chip> </gr-vote-chip>
+              <span class="overrideLabel"> Override </span>
+            </div>
+            <div class="votes-line">
+              <gr-vote-chip> </gr-vote-chip>
+              <span class="overrideLabel"> Override2 </span>
+            </div>
+          </div>
         </div>`
       );
     });
-- 
cgit v1.2.3