From 04e7db594bb007894f7d6f38258c353677f74697 Mon Sep 17 00:00:00 2001
From: Ian Tenney <iftenney@google.com>
Date: Tue, 20 Feb 2024 10:38:19 -0800
Subject: [PATCH] Use interstitial for all target sequence selection in LM
 salience.

Remove dropdown, and replace with a "Select sequence" button which re-opens the
interstitial selector.

PiperOrigin-RevId: 608659299
---
 lit_nlp/client/modules/lm_salience_module.css | 19 ++----
 lit_nlp/client/modules/lm_salience_module.ts  | 67 +++++++++----------
 2 files changed, 35 insertions(+), 51 deletions(-)

diff --git a/lit_nlp/client/modules/lm_salience_module.css b/lit_nlp/client/modules/lm_salience_module.css
index 91f51f06..8d874b4f 100644
--- a/lit_nlp/client/modules/lm_salience_module.css
+++ b/lit_nlp/client/modules/lm_salience_module.css
@@ -20,6 +20,7 @@
   text-overflow: ellipsis;
   overflow-x: hidden;
   line-height: 22px;
+  flex: 1;
 }
 
 lit-switch .icon-button {
@@ -48,13 +49,6 @@ select:invalid {
   padding: 4px 2px;  /* 2px left shift vs. smaller fonts below */
 }
 
-.interstitial-subtitle {
-  font-family: 'Google Sans', sans;
-  color: var(--lit-neutral-500);
-  padding: 4px;
-  margin-bottom: 8px;
-}
-
 .interstitial-contents {
   width: 80%;
   /* color: #5f6368; */
@@ -126,14 +120,9 @@ select:invalid {
   margin-right: 8px;
 }
 
-.target-dropdown-holder {
-  width: calc(100% - 22px);
-  margin-right: 4px;
-}
-
-.target-dropdown-holder .dropdown {
-  width: 100%;
-  text-overflow: ellipsis;
+.change-target-button {
+  white-space: nowrap;
+  margin: 0; /* have enough already from controls-group */
 }
 
 .vertical-separator {
diff --git a/lit_nlp/client/modules/lm_salience_module.ts b/lit_nlp/client/modules/lm_salience_module.ts
index 709604f5..20aaa0e3 100644
--- a/lit_nlp/client/modules/lm_salience_module.ts
+++ b/lit_nlp/client/modules/lm_salience_module.ts
@@ -661,52 +661,50 @@ export class LMSalienceModule extends SingleExampleSingleModelModule {
     }
   }
 
-  renderSalienceTargetStringSelector() {
-    const onChangeTarget = (e: Event) => {
-      const value = (e.target as HTMLInputElement).value;
-      this.salienceTargetOption = value !== '' ? +value : undefined;
-    };
+  renderSalienceTargetStringIndicator() {
+    const target = this.salienceTargetOption !== undefined ?
+        this.salienceTargetOptions[this.salienceTargetOption] :
+        null;
+    let sourceInfo = '';
+    let targetText = 'none selected.';
+    if (target != null) {
+      sourceInfo = target.source === TargetSource.REFERENCE ? ' (target)' :
+                                                              ' (response)';
+      targetText = target.text;
+    }
 
     const targetSelectorHelp =
         'Select a (response) from the model or a pre-defined (target) sequence from the dataset.';
 
-    const options = this.salienceTargetOptions.map((target, i) => {
-      // TODO(b/324959547): get field names 'target' and 'response' from spec
-      // via generated_text_utils.ts, rather than hard-coding.
-      // This information is available on the frontend, but we need to thread
-      // it through a few layers of code in generated_text_utils.ts
-      const sourceName =
-          target.source === TargetSource.REFERENCE ? 'target' : 'response';
-      // prettier-ignore
-      return html`
-          <option value=${i} ?selected=${i === this.salienceTargetOption}>
-             (${sourceName}) "${target.text}"
-          </option>`;
+    const isLoadingPreds = this.latestLoadPromises.has('modelPreds');
+
+    const indicatorTextClass = classMap({
+      'target-info-line': true,
+      'gray-text': target == null,
     });
-    // Empty default option. Styled as select:invalid.
-    // prettier-ignore
-    options.unshift(html`
-        <option value='' disabled hidden ?selected=${
-        this.salienceTargetOption === undefined}>
-          ${targetSelectorHelp}
-        </option>`);
 
-    const isLoadingPreds = this.latestLoadPromises.has('modelPreds');
+    const clearSalienceTarget = () => {
+      /* this will show the interstitial */
+      this.salienceTargetOption = undefined;
+    };
 
     // prettier-ignore
     return html`
       <div class="controls-group controls-group-variable"
         title=${targetSelectorHelp}>
-        <div class='target-dropdown-holder'>
-          <select required class='dropdown' @change=${onChangeTarget}>
-            ${options}
-          </select>
+        <label class="dropdown-label">Sequence${sourceInfo}:</label>
+        <div class=${indicatorTextClass}>
+          ${targetText}
           ${isLoadingPreds ? this.renderLoadingIndicator() : null}
         </div>
+      </div>
+      <div class='controls-group'>
         <lit-tooltip content=${targetSelectorHelp} tooltipPosition="left">
-          <span class="help-icon material-icon-outlined icon-button">
-            help_outline
-          </span>
+          <button class='hairline-button change-target-button'
+            slot='tooltip-anchor' @click=${clearSalienceTarget}
+            ?disabled=${target == null}>
+            <span>Select sequence </span><span class='material-icon'>arrow_drop_down</span>
+          </button>
         </lit-tooltip>
       </div>
     `;
@@ -843,9 +841,6 @@ export class LMSalienceModule extends SingleExampleSingleModelModule {
           <div class='interstitial-header'>
             Choose a sequence to explain
           </div>
-          <div class='interstitial-subtitle'>
-            Or select from the dropdown at the top of this module
-          </div>
           <div class='interstitial-target-selector'>
             <div class='interstitial-target-type'>From dataset (target):</div>
             ${optionsFromDataset}
@@ -956,7 +951,7 @@ export class LMSalienceModule extends SingleExampleSingleModelModule {
     return html`
       <div class="module-container">
         <div class="module-toolbar">
-          ${this.renderSalienceTargetStringSelector()}
+          ${this.renderSalienceTargetStringIndicator()}
         </div>
         <div class="module-toolbar">
           ${this.renderGranularitySelector()}