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

💄 render descriptionShort as markdown, fix DoD link display #2958

Merged
merged 2 commits into from
Nov 28, 2023
Merged

💄 render descriptionShort as markdown, fix DoD link display #2958

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
0d59680
💄 render descriptionShort as markdown, fix DoD link display
danyx23 Nov 28, 2023
cc2cd71
💄 make descriptionShort render without a paragraph to avoid whitespace
danyx23 Nov 28, 2023
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
5 changes: 5 additions & 0 deletions packages/@ourworldindata/grapher/src/modal/SourcesKeyDataTable.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@
color: inherit;
text-decoration: underline;

// DoD spans should be dotted via the border, not underlined
&.dod-span {
text-decoration: none;
}

&:hover {
text-decoration: none;
}
Expand Down
5 changes: 5 additions & 0 deletions packages/@ourworldindata/grapher/src/modal/SourcesModal.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@
&:hover {
text-decoration: none;
}

// DoD spans should be dotted via the border, not underlined
&.dod-span {
text-decoration: none;
}
}

.heading {
Expand Down
3 changes: 2 additions & 1 deletion packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import {
IndicatorSources,
IndicatorProcessing,
SimpleMarkdownText,
} from "@ourworldindata/components"
import React from "react"
import { action, computed } from "mobx"
Expand Down Expand Up @@ -393,7 +394,7 @@ export class Source extends React.Component<{
<div className="source">
{this.renderTitle()}
{this.def.descriptionShort && (
<p>{this.def.descriptionShort}</p>
<SimpleMarkdownText text={this.def.descriptionShort} />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sophiamersmann should we add the useParagraphs stanca here as well then?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the old code wraps descriptionShort in a paragraph as well, I would leave it as is here.

)}
<SourcesKeyDataTable
attribution={this.attributions}
Expand Down
5 changes: 5 additions & 0 deletions site/DataPageContent.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,11 @@
a {
@include owid-link-90;
color: inherit;

// DoD spans should be dotted via the border, not underlined
&.dod-span {
text-decoration: none;
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion site/DataPageV2Content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,12 @@ const KeyDataTable = (props: {
<div className="key-data__title key-data-description-short__title">
{datapageData.title}
</div>
<div>{datapageData.descriptionShort}</div>
<div>
<SimpleMarkdownText
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new renderer by default wraps any text into a paragraph which adds unwanted vertical space. I added an option called useParagraphs for single-liners specifically that – if set to false – disables that beaviour. It would be good to use that here.

<SimpleMarkdownText text={...} useParagraphs={false} />

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, alternatively, if we expect descriptionShort be have multiple paragraphs, then it would be good to set the p margins to 0 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know! The ETL guidelines say that this should be a single paragraph so I'll go with useParagraphs=false. Thank you!

text={datapageData.descriptionShort}
useParagraphs={false}
/>
</div>
</div>
)}
{source && (
Expand Down