Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change .dl-horizontal to be a grid #1077

Merged
merged 2 commits into from
Dec 9, 2024
Merged

Change .dl-horizontal to be a grid #1077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f665cf5
Change .dl-horizontal to be a grid
matthew-white Dec 5, 2024
aa2e9e3
Make changes after code review
matthew-white Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions src/assets/scss/app.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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; }
Expand Down
74 changes: 38 additions & 36 deletions src/components/entity/upload/header-errors.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -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; }
Expand Down
4 changes: 2 additions & 2 deletions test/components/entity/upload/header-errors.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
};
Expand All @@ -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', () => {
Expand Down