Skip to content

Commit d4e6790

Browse files
committed
fix(systemtags): only render inline list of tags if there are some
- resolves #59332 If there are no tags available, then we cannot render an `<ul>` element as this would result in invalid HTML / invalid accessibility state. Ref: https://www.w3.org/TR/wai-aria-1.2/#mustContain Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent 92e4c88 commit d4e6790

1 file changed

Lines changed: 54 additions & 51 deletions

File tree

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
}

0 commit comments

Comments
 (0)