Skip to content

fix: added the possibility of skipping lines for discord-embed titles #426

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

Merged
merged 6 commits into from
Jul 26, 2024
Merged

fix: added the possibility of skipping lines for discord-embed titles #426

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
12dd7d4
Adding the possibility of skipping lines
eumarciel404 Jul 26, 2024
59fe264
Remove "style="white-space: pre""
eumarciel404 Jul 26, 2024
a40eefe
style: prettier formatting
favna Jul 26, 2024
d3e8ea4
fix: follow our linting rules
favna Jul 26, 2024
07c0d6d
refactor: strictly type `el` and fix bugs
favna Jul 26, 2024
fd46751
refactor: change inline styles to a class
favna Jul 26, 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
53 changes: 40 additions & 13 deletions packages/core/src/components/discord-embed/DiscordEmbed.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { consume } from '@lit/context';
import { css, html, LitElement } from 'lit';
import { css, html, LitElement, type TemplateResult } from 'lit';
import { customElement, property } from 'lit/decorators.js';
import { classMap } from 'lit/directives/class-map.js';
import { ifDefined } from 'lit/directives/if-defined.js';
Expand Down Expand Up @@ -122,6 +122,11 @@ export class DiscordEmbed extends LitElement implements DiscordEmbedProps, Light
width: 24px;
}

:host .discord-embed-author-block,
:host .discord-embed-author-block > span {
max-width: 95%;
}

:host .discord-embed-provider {
font-size: 0.75rem;
line-height: 1rem;
Expand Down Expand Up @@ -328,10 +333,15 @@ export class DiscordEmbed extends LitElement implements DiscordEmbedProps, Light
${when(
this.authorUrl,
() =>
html`<a href=${ifDefined(this.authorUrl)} target="_blank" rel="noopener noreferrer">
${emojiParsedAuthorName}
html`<a
href=${ifDefined(this.authorUrl)}
target="_blank"
rel="noopener noreferrer"
class="discord-embed-author-block"
>
<span class="discord-embed-author-block">${emojiParsedAuthorName}</span>
</a>`,
() => html`${emojiParsedAuthorName}`
() => html`<span class="discord-embed-author-block">${emojiParsedAuthorName}</span>`
Comment on lines +340 to +344
Copy link
Member Author

Choose a reason for hiding this comment

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

You added inline styles here. That makes it harder for end-user customizability and we try to avoid them unless strictly necessary. I moved the styles to a class.

)}
</div>`
)}
Expand Down Expand Up @@ -388,18 +398,35 @@ export class DiscordEmbed extends LitElement implements DiscordEmbedProps, Light
private parseTitle(title?: string) {
if (!title) return null;

const words = title.split(' ');
const el: (TemplateResult<1> | string)[] = [];
Copy link
Member Author

@favna favna Jul 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This wasn't strongly typed, hiding a bug at the end of the function (see below)

let complete = '';

for (const words of title.split('\n')) {
for (const word of words.split(' ')) {
const emoji = getGlobalEmojiUrl(word) ?? this.embedEmojisMap[word] ?? ({} as Emoji);

if (emoji.name) {
el.push(html`<discord-custom-emoji name=${emoji.name} url=${ifDefined(emoji.url)} embed-emoji></discord-custom-emoji>`);
} else {
complete += `${word} `;
}

if (complete === ' ') {
el.push(html`<br />`);
}
}

el.push(complete);

complete = '';
}

return words.map((word: string, idx: number) => {
const emoji = getGlobalEmojiUrl(word) ?? this.embedEmojisMap[word] ?? ({} as Emoji);
let el;
if (emoji.name) {
el = html`<discord-custom-emoji name=${emoji.name} url=${ifDefined(emoji.url)} embed-emoji></discord-custom-emoji>`;
} else {
el = idx < words.length - 1 ? `${word} ` : word;
return el.map((wordOrHtmlTemplate) => {
if (typeof wordOrHtmlTemplate === 'string') {
return html`<span>${wordOrHtmlTemplate}</span>`;
}

return el;
return wordOrHtmlTemplate;
});
Comment on lines +424 to 430
Copy link
Member Author

@favna favna Jul 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In your code you typed word (now wordOrHtmlTemplate) as string, I assume this was because it was typed as any above. This hid a bug however, because in one of the code branches, it is not a string but an HTML TemplateResult with discord-custom-emoji and in that case, it should not be wrapped with another TemplateResult holding a span. By strictly typing el this is revealed more clearly and you can add a typeof check to ensure that only raw strings get wrapped with a span.

}
}
Expand Down