Skip to content

fix(systemtags): only render inline list of tags if there are some #59424

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

Open
susnux wants to merge 1 commit into
base: master
Choose a base branch
from
+64 −61
Open

fix(systemtags): only render inline list of tags if there are some #59424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 10 additions & 10 deletions apps/systemtags/src/files_actions/inlineSystemTagsAction.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('Inline system tags action render tests', () => {
contents: [],
})
expect(result).toBeInstanceOf(HTMLElement)
expect(result!.outerHTML).toMatchInlineSnapshot('"<ul class="files-list__system-tags" aria-label="Assigned collaborative tags" data-systemtags-fileid="1"></ul>"')
expect(result!.outerHTML).toMatchInlineSnapshot('"<div data-systemtags-fileid="1"></div>"')
})

test('Render a single system tag', async () => {
Expand All @@ -126,7 +126,7 @@ describe('Inline system tags action render tests', () => {
contents: [],
})
expect(result).toBeInstanceOf(HTMLElement)
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>"')
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>"')
})

test('Render two system tags', async () => {
Expand All @@ -151,7 +151,7 @@ describe('Inline system tags action render tests', () => {
contents: [],
})
expect(result).toBeInstanceOf(HTMLElement)
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>"')
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>"')
})

test('Render multiple system tags', async () => {
Expand Down Expand Up @@ -181,7 +181,7 @@ describe('Inline system tags action render tests', () => {
contents: [],
})
expect(result).toBeInstanceOf(HTMLElement)
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>"')
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>"')
})

test('Render gets updated on system tag change', async () => {
Expand Down Expand Up @@ -212,7 +212,7 @@ describe('Inline system tags action render tests', () => {
}) as HTMLElement
document.body.appendChild(result)
expect(result).toBeInstanceOf(HTMLElement)
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>"')
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>"')

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

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>"')
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>"')
})
})

Expand Down Expand Up @@ -273,7 +273,7 @@ describe('Inline system tags action colors', () => {
contents: [],
})
expect(result).toBeInstanceOf(HTMLElement)
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>"')
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>"')
})

test('Render a single system tag with invalid WCAG color', async () => {
Expand All @@ -300,7 +300,7 @@ describe('Inline system tags action colors', () => {
contents: [],
})
expect(result).toBeInstanceOf(HTMLElement)
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>"')
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>"')

document.body.removeAttribute('data-themes')
})
Expand Down Expand Up @@ -328,7 +328,7 @@ describe('Inline system tags action colors', () => {
}) as HTMLElement
document.body.appendChild(result)
expect(result).toBeInstanceOf(HTMLElement)
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>"')
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>"')

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

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>"')
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>"')
})
})
105 changes: 54 additions & 51 deletions apps/systemtags/src/files_actions/inlineSystemTagsAction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,15 @@ export const action: IFileAction = {
return true
},

exec: async () => null,
renderInline: ({ nodes }) => {
async exec() {
return null
},

async renderInline({ nodes }) {
if (nodes.length !== 1 || !nodes[0]) {
return Promise.resolve(null)
return null
}
return renderInline(nodes[0])
return await renderInline(nodes[0])
},

order: 0,
Expand All @@ -56,12 +59,12 @@ subscribe('systemtags:tag:updated', updateTag)
*
* @param node - The updated node
*/
function updateSystemTagsHtml(node: INode) {
renderInline(node).then((systemTagsHtml) => {
document.querySelectorAll(`[data-systemtags-fileid="${node.fileid}"]`).forEach((element) => {
element.replaceWith(systemTagsHtml)
})
})
async function updateSystemTagsHtml(node: INode) {
const systemTagsHtml = await renderInline(node)
const elements = document.querySelectorAll(`[data-systemtags-fileid="${node.id}"]`)
for (const element of elements) {
element.replaceWith(systemTagsHtml)
}
}

/**
Expand Down Expand Up @@ -145,50 +148,50 @@ function renderTag(tag: string, isMore = false): HTMLElement {
async function renderInline(node: INode): Promise<HTMLElement> {
// Ensure we have the system tags as an array
const tags = getNodeSystemTags(node)

const systemTagsElement = document.createElement('ul')
systemTagsElement.classList.add('files-list__system-tags')
systemTagsElement.setAttribute('aria-label', t('files', 'Assigned collaborative tags'))
systemTagsElement.setAttribute('data-systemtags-fileid', node.fileid?.toString() || '')

if (tags.length === 0) {
return systemTagsElement
}

// Fetch the tags if the cache is empty
if (cache.length === 0) {
try {
// Best would be to support attributes from webdav,
// but currently the library does not support it
cache.push(...await fetchTags())
} catch (error) {
logger.error('Failed to fetch tags', { error })
const systemTagsElementWrapper = document.createElement('div')
systemTagsElementWrapper.setAttribute('data-systemtags-fileid', node.id || '')

if (tags.length > 0) {
const systemTagsElement = document.createElement('ul')
systemTagsElement.classList.add('files-list__system-tags')
systemTagsElement.setAttribute('aria-label', t('files', 'Assigned collaborative tags'))
systemTagsElementWrapper.appendChild(systemTagsElement)

// Fetch the tags if the cache is empty
if (cache.length === 0) {
try {
// Best would be to support attributes from webdav,
// but currently the library does not support it
cache.push(...await fetchTags())
} catch (error) {
logger.error('Failed to fetch tags', { error })
}
}
}

systemTagsElement.append(renderTag(tags[0]!))
if (tags.length === 2) {
// Special case only two tags:
// the overflow fake tag would take the same space as this, so render it
systemTagsElement.append(renderTag(tags[1]!))
} else if (tags.length > 1) {
// More tags than the one we're showing
// So we add a overflow element indicating there are more tags
const moreTagElement = renderTag('+' + (tags.length - 1), true)
moreTagElement.setAttribute('title', tags.slice(1).join(', '))
// because the title is not accessible we hide this element for screen readers (see alternative below)
moreTagElement.setAttribute('aria-hidden', 'true')
moreTagElement.setAttribute('role', 'presentation')
systemTagsElement.append(moreTagElement)

// For accessibility the tags are listed, as the title is not accessible
// but those tags are visually hidden
for (const tag of tags.slice(1)) {
const tagElement = renderTag(tag)
tagElement.classList.add('hidden-visually')
systemTagsElement.append(tagElement)
systemTagsElement.append(renderTag(tags[0]!))
if (tags.length === 2) {
// Special case only two tags:
// the overflow fake tag would take the same space as this, so render it
systemTagsElement.append(renderTag(tags[1]!))
} else if (tags.length > 1) {
// More tags than the one we're showing
// So we add a overflow element indicating there are more tags
const moreTagElement = renderTag('+' + (tags.length - 1), true)
moreTagElement.setAttribute('title', tags.slice(1).join(', '))
// because the title is not accessible we hide this element for screen readers (see alternative below)
moreTagElement.setAttribute('aria-hidden', 'true')
moreTagElement.setAttribute('role', 'presentation')
systemTagsElement.append(moreTagElement)

// For accessibility the tags are listed, as the title is not accessible
// but those tags are visually hidden
for (const tag of tags.slice(1)) {
const tagElement = renderTag(tag)
tagElement.classList.add('hidden-visually')
systemTagsElement.append(tagElement)
}
}
}

return systemTagsElement
return systemTagsElementWrapper
}
Loading