Change .dl-horizontal to be a grid (#1077)

getodk · Dec 9, 2024 · 55217ea · 55217ea
1 parent c259f04
commit 55217ea
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 49 deletions.
diff --git a/src/assets/scss/app.scss b/src/assets/scss/app.scss
@@ -40,21 +40,26 @@ h1, .h1 {
 
 p { @include text-block; }
 
-dl > div {
+dl > * {
   border-bottom: 1px solid #ddd;
-  display: flex;
   padding-bottom: 10px;
   padding-top: 10px;
 
-  &:first-child { padding-top: 0; }
+  &:first-of-type { padding-top: 0; }
 
-  &:last-child {
+  &:last-of-type {
     border-bottom: none;
     padding-bottom: 0;
   }
 }
+
+// Normal <dl> elements that are not .dl-horizontal are expected to have
+// children that are <div> elements. Each <div> must contain a <dt> and a <dd>.
 dl:not(.dl-horizontal) {
-  > div { flex-direction: column-reverse; }
+  > div {
+    display: flex;
+    flex-direction: column-reverse;
+  }
 
   dt {
     color: #666;
@@ -63,14 +68,28 @@ dl:not(.dl-horizontal) {
 
   dd { font-size: 16px; }
 }
+
+// <dl> elements that are .dl-horizontal are expected to have children that are
+// <dt> and <dd> elements. That's different from how other <dl> elements work,
+// as their children are expected to be <div> elements. The difference comes
+// down to the fact that .dl-horizontal uses a CSS grid, while other <dl>
+// elements use flexbox.
 .dl-horizontal {
-  dt {
-    @include text-overflow-ellipsis;
-    flex-shrink: 0;
-    width: 160px;
-  }
+  $padding-between: 10px;
+  $dt-width: 160px + $padding-panel-body + $padding-between;
+
+  display: grid;
+  // Components may override grid-template-columns in order to adjust the width
+  // of the <dt>.
+  grid-template-columns: $dt-width 1fr;
+
+  dt { padding-left: $padding-panel-body; }
+  dd { padding-right: $padding-panel-body; }
 
-  dd { margin-left: 20px; }
+  // Equally distribute the padding between the <dt> and the <dd> so that it is
+  // easy for components to give the two elements the same width.
+  dt { padding-right: $padding-between; }
+  dd { padding-left: $padding-between; }
 }
 
 .dfn { border-bottom: 2px dotted #999; }

diff --git a/src/components/entity/upload/header-errors.vue b/src/components/entity/upload/header-errors.vue
@@ -11,38 +11,42 @@ except according to the terms contained in the LICENSE file.
 -->
 <template>
   <dl id="entity-upload-header-errors" class="dl-horizontal">
-    <div>
-      <dt>{{ $t('expectedHeader') }}</dt>
-      <dd :ref="setHeaderElement(0)" class="csv-header" @scroll="scrollHeader">
+    <dt>{{ $t('expectedHeader') }}</dt>
+    <dd>
+      <div :ref="setHeaderElement(0)" class="csv-header" @scroll="scrollHeader">
         {{ expectedHeader }}
-      </dd>
-    </div>
-    <div>
-      <dt v-tooltip.text>{{ filename }}</dt>
-      <dd :ref="setHeaderElement(1)" class="csv-header" @scroll="scrollHeader">
+      </div>
+    </dd>
+
+    <dt id="entity-upload-header-errors-filename" v-tooltip.text>
+      {{ filename }}
+    </dt>
+    <dd>
+      <div :ref="setHeaderElement(1)" class="csv-header" @scroll="scrollHeader">
         {{ header }}
-      </dd>
-    </div>
-    <div v-if="hasSuggestion" id="entity-upload-header-errors-suggestions">
-      <dt>{{ $t('suggestions.title') }}</dt>
-      <dd>
-        <p v-if="invalidQuotes">{{ $t('suggestions.invalidQuotes') }}</p>
-        <i18n-t v-if="missingLabel" tag="p" keypath="suggestions.missingLabel">
-          <template #label>
-            <span class="text-monospace">label</span>
-          </template>
-        </i18n-t>
-        <p v-if="unknownProperty">{{ $t('suggestions.unknownProperty') }}</p>
-        <p v-if="duplicateColumn">{{ $t('suggestions.duplicateColumn') }}</p>
-        <p v-if="emptyColumn">{{ $t('suggestions.emptyColumn') }}</p>
-        <i18n-t v-if="delimiter !== ','" tag="p"
-          keypath="suggestions.delimiterNotComma">
-          <template #delimiter>
-            <code>{{ formattedDelimiter }}</code>
-          </template>
-        </i18n-t>
-      </dd>
-    </div>
+      </div>
+    </dd>
+
+    <dt v-if="hasSuggestion" class="entity-upload-header-errors-suggestions">
+      {{ $t('suggestions.title') }}
+    </dt>
+    <dd v-if="hasSuggestion" class="entity-upload-header-errors-suggestions">
+      <p v-if="invalidQuotes">{{ $t('suggestions.invalidQuotes') }}</p>
+      <i18n-t v-if="missingLabel" tag="p" keypath="suggestions.missingLabel">
+        <template #label>
+          <span class="text-monospace">label</span>
+        </template>
+      </i18n-t>
+      <p v-if="unknownProperty">{{ $t('suggestions.unknownProperty') }}</p>
+      <p v-if="duplicateColumn">{{ $t('suggestions.duplicateColumn') }}</p>
+      <p v-if="emptyColumn">{{ $t('suggestions.emptyColumn') }}</p>
+      <i18n-t v-if="delimiter !== ','" tag="p"
+        keypath="suggestions.delimiterNotComma">
+        <template #delimiter>
+          <code>{{ formattedDelimiter }}</code>
+        </template>
+      </i18n-t>
+    </dd>
   </dl>
 </template>
 
@@ -108,20 +112,18 @@ const formattedDelimiter = computed(() => formatCSVDelimiter(props.delimiter));
 #entity-upload-header-errors {
   margin-bottom: 0;
 
-  div {
-    padding-left: $padding-panel-body;
-    padding-right: $padding-panel-body;
-  }
-
   .csv-header {
     @include line-clamp(3);
     font-family: $font-family-monospace;
     overflow-x: auto;
     white-space: break-spaces;
   }
+  dd:has(.csv-header) { overflow-x: hidden; }
 }
 
-#entity-upload-header-errors-suggestions {
+#entity-upload-header-errors-filename { @include text-overflow-ellipsis; }
+
+.entity-upload-header-errors-suggestions {
   color: $color-danger;
 
   p:last-child { margin-bottom: 0; }

diff --git a/test/components/entity/upload/header-errors.spec.js b/test/components/entity/upload/header-errors.spec.js
@@ -36,7 +36,7 @@ describe('EntityUploadHeaderErrors', () => {
 
   describe('suggestions', () => {
     const getSuggestion = (component) => {
-      const p = component.findAll('#entity-upload-header-errors-suggestions p');
+      const p = component.findAll('.entity-upload-header-errors-suggestions p');
       p.length.should.equal(1);
       return p[0];
     };
@@ -48,7 +48,7 @@ describe('EntityUploadHeaderErrors', () => {
       const component = mountComponent({
         props: { header: 'label', missingProperty: true }
       });
-      component.find('#entity-upload-header-errors-suggestions').exists().should.be.false;
+      component.find('.entity-upload-header-errors-suggestions').exists().should.be.false;
     });
 
     describe('delimiter is not a comma', () => {