rich_content: overhaul spacing around mentions
The old code was inconsistent about when the mention line state and remembered spacing were reset as well as failing to add necessary whitespace if a mention line was immediately succeeded by a plain text node. The latter was kludgily fixd for a particular special case with the last-child exception to whitespace trimming. (However this could en up to retaining superfluous undesireable space in other cases) This led to sometimes space being added "back" several times notably leading to one extra, empty line in blockquote elements succeeding mentions with whitespace. This stalled the otherwise unrelated #412 Now we always reset the mention chain and remembered spacing together and also add back spacing in front of plain text nodes if appropriate obsoleting the last-child exception.
This commit is contained in:
parent
2c252bb6bf
commit
c7d7fd5fdd
3 changed files with 80 additions and 11 deletions
|
|
@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
|
|||
- fix notifications on mobile
|
||||
- fix attachment display for remotes not federating any MIME type indicators if they still indicate a generic type.
|
||||
This applies to e.g. bridgy
|
||||
- fixed some spacing issues after mentions
|
||||
|
||||
### Changed
|
||||
- reworked rich content (anything with custom emoji or not pure plaintext) parsing;
|
||||
|
|
|
|||
|
|
@ -208,6 +208,11 @@ class RichContentParser {
|
|||
/>
|
||||
}
|
||||
|
||||
#breakMentionChain() {
|
||||
this.#currentMentions = []
|
||||
this.#lastSpacing = ''
|
||||
}
|
||||
|
||||
#maybePushTextNode(acc, text, start, end) {
|
||||
if (end > start && start >= 0) {
|
||||
acc.push(text.slice(start, end))
|
||||
|
|
@ -250,26 +255,28 @@ class RichContentParser {
|
|||
}
|
||||
|
||||
#processTextNode(textNode) {
|
||||
const text = textNode.textContent;
|
||||
let text = textNode.textContent;
|
||||
const isEmpty = text.trim() === ''
|
||||
|
||||
if (isEmpty) {
|
||||
// No idea why the old code did that; in-source linebreaks have no special meaning in HTML.
|
||||
// Maybe a quirk of the cursed old NIH parser, but doesn't make any sense to me now.
|
||||
//if (text.includes('\n')) this.#currentMentions = [];
|
||||
//if (text.includes('\n')) this.#breakMentionChain();
|
||||
|
||||
// don't include spaces between consecutive mentions.
|
||||
// we'll include them in MentionsLine.
|
||||
// However, do preserve _trailing_ whitespace, in case the original source
|
||||
// placed the spacing between mentions and following elements within the mention context.
|
||||
if (this.#currentMentions.length > 0 && textNode.parentNode?.lastChild !== textNode) {
|
||||
// Don't include spaces between consecutive mentions. Since Mentions are inserted into
|
||||
// the preceeding MentionLine element, we’ll end up with superfluous whitespace otherwise.
|
||||
if (this.#currentMentions.length > 0) {
|
||||
this.#lastSpacing = text
|
||||
textNode.textContent = ''
|
||||
}
|
||||
return [textNode.textContent]
|
||||
}
|
||||
|
||||
this.#currentMentions = []
|
||||
if (this.#lastSpacing && !text.match(/^\s/)) {
|
||||
text = this.#lastSpacing + text
|
||||
}
|
||||
|
||||
this.#breakMentionChain()
|
||||
|
||||
return this.#processTextForEmoji(text)
|
||||
}
|
||||
|
|
@ -319,7 +326,7 @@ class RichContentParser {
|
|||
}
|
||||
|
||||
// mention chain no longer consecutive
|
||||
this.#currentMentions = []
|
||||
this.#breakMentionChain()
|
||||
|
||||
// Hashtags
|
||||
if (
|
||||
|
|
@ -352,10 +359,11 @@ class RichContentParser {
|
|||
|
||||
switch (Tag) {
|
||||
case 'BR':
|
||||
this.#currentMentions = []
|
||||
this.#breakMentionChain()
|
||||
return [<br />]
|
||||
|
||||
case 'IMG':
|
||||
this.#breakMentionChain()
|
||||
return [mentionsLinePadding, this.#renderImage(node)]
|
||||
|
||||
case 'A':
|
||||
|
|
@ -370,6 +378,8 @@ class RichContentParser {
|
|||
break
|
||||
}
|
||||
|
||||
this.#breakMentionChain()
|
||||
|
||||
const pchilds = [...node.childNodes].map(n => this.#processNode(n))
|
||||
const attrs = this.#nodeAttributeMap(node)
|
||||
return [mentionsLinePadding, <Tag {...attrs}>{ pchilds }</Tag>]
|
||||
|
|
|
|||
|
|
@ -499,8 +499,8 @@ describe('RichContent', () => {
|
|||
'</span>',
|
||||
'<!--v-if-->', // v-if placeholder, mentionsline's extra mentions and stuff
|
||||
'</span>',
|
||||
' ',
|
||||
'</span>',
|
||||
' ',
|
||||
'Testing'
|
||||
].join('')
|
||||
|
||||
|
|
@ -518,6 +518,64 @@ describe('RichContent', () => {
|
|||
expect(wrapper.html().replace(/\n/g, '')).to.eql(compwrap(expected))
|
||||
})
|
||||
|
||||
it.skip('does not add back whitespace twice or too late', () => {
|
||||
// This test is skipped due to what appears to be a bug in Vue’s test-utils
|
||||
// refer to https://akkoma.dev/AkkomaGang/akkoma-fe/pulls/503 for details
|
||||
// (if upgrading Vue/test-utils ideally check if the test now works and enable if if it does)
|
||||
const html_mention = [
|
||||
'<span class="h-card">',
|
||||
'<a class="u-url mention" href="https://example.org/users/user" rel="ugc">',
|
||||
'@<span>user</span>',
|
||||
'</a>',
|
||||
'</span>'
|
||||
].join('')
|
||||
|
||||
const html = [
|
||||
'<p>',
|
||||
html_mention,
|
||||
' ',
|
||||
html_mention,
|
||||
' ',
|
||||
'</p>',
|
||||
'<blockquote><p>aa<br/>bb</p></blockquote>'
|
||||
].join('')
|
||||
|
||||
const expected_mentionlink = [
|
||||
'<span class="MentionLink mention-link">',
|
||||
'<!-- eslint-disable vue/no-v-html -->',
|
||||
'<a href="https://example.org/users/user" class="original" target="_blank">',
|
||||
'@<span>user</span>',
|
||||
'</a>',
|
||||
'<!-- eslint-enable vue/no-v-html --><!--v-if-->',
|
||||
'</span>'
|
||||
].join('')
|
||||
|
||||
const expected = [
|
||||
'<p>',
|
||||
'<span class="MentionsLine">',
|
||||
expected_mentionlink,
|
||||
expected_mentionlink,
|
||||
'<!--v-if-->',
|
||||
'</span>',
|
||||
'</p>',
|
||||
' ',
|
||||
'<blockquote><p>aa<br>bb</p></blockquote>'
|
||||
].join('')
|
||||
|
||||
const wrapper = mount(RichContent, {
|
||||
global,
|
||||
props: {
|
||||
attentions,
|
||||
handleLinks: true,
|
||||
greentext: true,
|
||||
emoji: [],
|
||||
html
|
||||
}
|
||||
})
|
||||
|
||||
expect(wrapper.html().replace(/\n/g, '')).to.eql(compwrap(expected))
|
||||
})
|
||||
|
||||
it('rich contents of a link are handled properly', () => {
|
||||
const html = [
|
||||
'<p>',
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue