Skip to content

Commit 13583e8

Browse files
authored
Merge pull request #59424 from nextcloud/fix/systemtags-inline
fix(systemtags): only render inline list of tags if there are some
2 parents 0526c18 + b9e4a2a commit 13583e8

File tree

4 files changed

+67
-64
lines changed

4 files changed

+67
-64
lines changed

apps/systemtags/src/files_actions/inlineSystemTagsAction.spec.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ describe('Inline system tags action render tests', () => {
101101
contents: [],
102102
})
103103
expect(result).toBeInstanceOf(HTMLElement)
104-
expect(result!.outerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"></ul>"')
104+
expect(result!.outerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"></div>"')
105105
})
106106

107107
test('Render a single system tag', async () => {
@@ -126,7 +126,7 @@ describe('Inline system tags action render tests', () => {
126126
contents: [],
127127
})
128128
expect(result).toBeInstanceOf(HTMLElement)
129-
expect(result!.outerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag" data-systemtag-name="Confidential">Confidential</li></ul>"')
129+
expect(result!.outerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"><ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag" data-systemtag-name="Confidential">Confidential</li></ul></div>"')
130130
})
131131

132132
test('Render two system tags', async () => {
@@ -151,7 +151,7 @@ describe('Inline system tags action render tests', () => {
151151
contents: [],
152152
})
153153
expect(result).toBeInstanceOf(HTMLElement)
154-
expect(result!.outerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag" data-systemtag-name="Important">Important</li><li class="files-list__system-tag" data-systemtag-name="Confidential">Confidential</li></ul>"')
154+
expect(result!.outerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"><ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag" data-systemtag-name="Important">Important</li><li class="files-list__system-tag" data-systemtag-name="Confidential">Confidential</li></ul></div>"')
155155
})
156156

157157
test('Render multiple system tags', async () => {
@@ -181,7 +181,7 @@ describe('Inline system tags action render tests', () => {
181181
contents: [],
182182
})
183183
expect(result).toBeInstanceOf(HTMLElement)
184-
expect(result!.outerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag" data-systemtag-name="Important">Important</li><li class="files-list__system-tag files-list__system-tag--more" data-systemtag-name="+3" title="Confidential, Secret, Classified" aria-hidden="true" role="presentation">+3</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Confidential">Confidential</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Secret">Secret</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Classified">Classified</li></ul>"')
184+
expect(result!.outerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"><ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag" data-systemtag-name="Important">Important</li><li class="files-list__system-tag files-list__system-tag--more" data-systemtag-name="+3" title="Confidential, Secret, Classified" aria-hidden="true" role="presentation">+3</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Confidential">Confidential</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Secret">Secret</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Classified">Classified</li></ul></div>"')
185185
})
186186

187187
test('Render gets updated on system tag change', async () => {
@@ -212,7 +212,7 @@ describe('Inline system tags action render tests', () => {
212212
}) as HTMLElement
213213
document.body.appendChild(result)
214214
expect(result).toBeInstanceOf(HTMLElement)
215-
expect(document.body.innerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag" data-systemtag-name="Important">Important</li><li class="files-list__system-tag files-list__system-tag--more" data-systemtag-name="+3" title="Confidential, Secret, Classified" aria-hidden="true" role="presentation">+3</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Confidential">Confidential</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Secret">Secret</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Classified">Classified</li></ul>"')
215+
expect(document.body.innerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"><ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag" data-systemtag-name="Important">Important</li><li class="files-list__system-tag files-list__system-tag--more" data-systemtag-name="+3" title="Confidential, Secret, Classified" aria-hidden="true" role="presentation">+3</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Confidential">Confidential</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Secret">Secret</li><li class="files-list__system-tag hidden-visually" data-systemtag-name="Classified">Classified</li></ul></div>"')
216216

217217
// Subscribe to the event
218218
const eventPromise = new Promise((resolve) => {
@@ -229,7 +229,7 @@ describe('Inline system tags action render tests', () => {
229229
// Wait for the event to be processed
230230
await eventPromise
231231

232-
expect(document.body.innerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag" data-systemtag-name="Public">Public</li></ul>"')
232+
expect(document.body.innerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"><ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag" data-systemtag-name="Public">Public</li></ul></div>"')
233233
})
234234
})
235235

@@ -273,7 +273,7 @@ describe('Inline system tags action colors', () => {
273273
contents: [],
274274
})
275275
expect(result).toBeInstanceOf(HTMLElement)
276-
expect(result!.outerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag" data-systemtag-name="Confidential" style="--systemtag-color: #000000;" data-systemtag-color="true">Confidential</li></ul>"')
276+
expect(result!.outerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"><ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag" data-systemtag-name="Confidential" style="--systemtag-color: #000000;" data-systemtag-color="true">Confidential</li></ul></div>"')
277277
})
278278

279279
test('Render a single system tag with invalid WCAG color', async () => {
@@ -300,7 +300,7 @@ describe('Inline system tags action colors', () => {
300300
contents: [],
301301
})
302302
expect(result).toBeInstanceOf(HTMLElement)
303-
expect(result!.outerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag" data-systemtag-name="Confidential" style="--systemtag-color: #646464;" data-systemtag-color="true">Confidential</li></ul>"')
303+
expect(result!.outerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"><ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag" data-systemtag-name="Confidential" style="--systemtag-color: #646464;" data-systemtag-color="true">Confidential</li></ul></div>"')
304304

305305
document.body.removeAttribute('data-themes')
306306
})
@@ -328,7 +328,7 @@ describe('Inline system tags action colors', () => {
328328
}) as HTMLElement
329329
document.body.appendChild(result)
330330
expect(result).toBeInstanceOf(HTMLElement)
331-
expect(document.body.innerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag" data-systemtag-name="Confidential" style="--systemtag-color: #000000;" data-systemtag-color="true">Confidential</li></ul>"')
331+
expect(document.body.innerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"><ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag" data-systemtag-name="Confidential" style="--systemtag-color: #000000;" data-systemtag-color="true">Confidential</li></ul></div>"')
332332

333333
// Subscribe to the event
334334
const eventPromise = new Promise((resolve) => {
@@ -344,6 +344,6 @@ describe('Inline system tags action colors', () => {
344344
// Wait for the event to be processed
345345
await eventPromise
346346

347-
expect(document.body.innerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"><li class="files-list__system-tag" data-systemtag-name="Confidential" style="--systemtag-color: #456789;" data-systemtag-color="true">Confidential</li></ul>"')
347+
expect(document.body.innerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"><ul class="files-list__system-tags" aria-label="Assigned collaborative tags"><li class="files-list__system-tag" data-systemtag-name="Confidential" style="--systemtag-color: #456789;" data-systemtag-color="true">Confidential</li></ul></div>"')
348348
})
349349
})

apps/systemtags/src/files_actions/inlineSystemTagsAction.ts

Lines changed: 54 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,15 @@ export const action: IFileAction = {
3434
return true
3535
},
3636

37-
exec: async () => null,
38-
renderInline: ({ nodes }) => {
37+
async exec() {
38+
return null
39+
},
40+
41+
async renderInline({ nodes }) {
3942
if (nodes.length !== 1 || !nodes[0]) {
40-
return Promise.resolve(null)
43+
return null
4144
}
42-
return renderInline(nodes[0])
45+
return await renderInline(nodes[0])
4346
},
4447

4548
order: 0,
@@ -56,12 +59,12 @@ subscribe('systemtags:tag:updated', updateTag)
5659
*
5760
* @param node - The updated node
5861
*/
59-
function updateSystemTagsHtml(node: INode) {
60-
renderInline(node).then((systemTagsHtml) => {
61-
document.querySelectorAll(`[data-systemtags-fileid="${node.fileid}"]`).forEach((element) => {
62-
element.replaceWith(systemTagsHtml)
63-
})
64-
})
62+
async function updateSystemTagsHtml(node: INode) {
63+
const systemTagsHtml = await renderInline(node)
64+
const elements = document.querySelectorAll(`[data-systemtags-fileid="${node.id}"]`)
65+
for (const element of elements) {
66+
element.replaceWith(systemTagsHtml)
67+
}
6568
}
6669

6770
/**
@@ -145,50 +148,50 @@ function renderTag(tag: string, isMore = false): HTMLElement {
145148
async function renderInline(node: INode): Promise<HTMLElement> {
146149
// Ensure we have the system tags as an array
147150
const tags = getNodeSystemTags(node)
148-
149-
const systemTagsElement = document.createElement('ul')
150-
systemTagsElement.classList.add('files-list__system-tags')
151-
systemTagsElement.setAttribute('aria-label', t('files', 'Assigned collaborative tags'))
152-
systemTagsElement.setAttribute('data-systemtags-fileid', node.fileid?.toString() || '')
153-
154-
if (tags.length === 0) {
155-
return systemTagsElement
156-
}
157-
158-
// Fetch the tags if the cache is empty
159-
if (cache.length === 0) {
160-
try {
161-
// Best would be to support attributes from webdav,
162-
// but currently the library does not support it
163-
cache.push(...await fetchTags())
164-
} catch (error) {
165-
logger.error('Failed to fetch tags', { error })
151+
const systemTagsElementWrapper = document.createElement('div')
152+
systemTagsElementWrapper.setAttribute('data-systemtags-fileid', node.id || '')
153+
154+
if (tags.length > 0) {
155+
const systemTagsElement = document.createElement('ul')
156+
systemTagsElement.classList.add('files-list__system-tags')
157+
systemTagsElement.setAttribute('aria-label', t('files', 'Assigned collaborative tags'))
158+
systemTagsElementWrapper.appendChild(systemTagsElement)
159+
160+
// Fetch the tags if the cache is empty
161+
if (cache.length === 0) {
162+
try {
163+
// Best would be to support attributes from webdav,
164+
// but currently the library does not support it
165+
cache.push(...await fetchTags())
166+
} catch (error) {
167+
logger.error('Failed to fetch tags', { error })
168+
}
166169
}
167-
}
168170

169-
systemTagsElement.append(renderTag(tags[0]!))
170-
if (tags.length === 2) {
171-
// Special case only two tags:
172-
// the overflow fake tag would take the same space as this, so render it
173-
systemTagsElement.append(renderTag(tags[1]!))
174-
} else if (tags.length > 1) {
175-
// More tags than the one we're showing
176-
// So we add a overflow element indicating there are more tags
177-
const moreTagElement = renderTag('+' + (tags.length - 1), true)
178-
moreTagElement.setAttribute('title', tags.slice(1).join(', '))
179-
// because the title is not accessible we hide this element for screen readers (see alternative below)
180-
moreTagElement.setAttribute('aria-hidden', 'true')
181-
moreTagElement.setAttribute('role', 'presentation')
182-
systemTagsElement.append(moreTagElement)
183-
184-
// For accessibility the tags are listed, as the title is not accessible
185-
// but those tags are visually hidden
186-
for (const tag of tags.slice(1)) {
187-
const tagElement = renderTag(tag)
188-
tagElement.classList.add('hidden-visually')
189-
systemTagsElement.append(tagElement)
171+
systemTagsElement.append(renderTag(tags[0]!))
172+
if (tags.length === 2) {
173+
// Special case only two tags:
174+
// the overflow fake tag would take the same space as this, so render it
175+
systemTagsElement.append(renderTag(tags[1]!))
176+
} else if (tags.length > 1) {
177+
// More tags than the one we're showing
178+
// So we add a overflow element indicating there are more tags
179+
const moreTagElement = renderTag('+' + (tags.length - 1), true)
180+
moreTagElement.setAttribute('title', tags.slice(1).join(', '))
181+
// because the title is not accessible we hide this element for screen readers (see alternative below)
182+
moreTagElement.setAttribute('aria-hidden', 'true')
183+
moreTagElement.setAttribute('role', 'presentation')
184+
systemTagsElement.append(moreTagElement)
185+
186+
// For accessibility the tags are listed, as the title is not accessible
187+
// but those tags are visually hidden
188+
for (const tag of tags.slice(1)) {
189+
const tagElement = renderTag(tag)
190+
tagElement.classList.add('hidden-visually')
191+
systemTagsElement.append(tagElement)
192+
}
190193
}
191194
}
192195

193-
return systemTagsElement
196+
return systemTagsElementWrapper
194197
}

dist/systemtags-init.mjs

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

dist/systemtags-init.mjs.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)