Skip to content
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.

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

fix(LogTable): implement virtual scrolling #1505

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion css/logreader-main.css
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
/* extracted by css-entry-points-plugin */
@import './main-KB4pBeS4.chunk.css';
@import './main-Dq-n66Va.chunk.css';
16 changes: 16 additions & 0 deletions css/main-Dq-n66Va.chunk.css

Large diffs are not rendered by default.

76 changes: 38 additions & 38 deletions js/logreader-main.mjs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/logreader-main.mjs.map

Large diffs are not rendered by default.

162 changes: 127 additions & 35 deletions src/components/table/LogTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
:open.sync="isModalOpen"
:current-entry.sync="currentRow"
:log-entries="sortedRows" />
<table class="log-table__table">
<thead>
<table ref="tableRoot" class="log-table__table">
<thead role="rowgroup" class="log-table__header">
<tr>
<LogTableHeader :name="t('logreader', 'Level')"
:sorted.sync="sortedByLevel" />
Expand All @@ -23,9 +23,9 @@
<th><span class="hidden-visually">{{ t('logreader', 'Log entry actions') }}</span></th>
</tr>
</thead>
<tbody ref="tableBody">
<tr v-if="sortedByTime === 'ascending'">
<td colspan="5" class="log-table__load-more">
<tbody ref="tableBody" :style="tbodyStyle" class="log-table__body">
<tr v-if="sortedByTime === 'ascending'" class="log-table__load-more">
<td>
<IntersectionObserver v-if="logStore.hasRemainingEntries" @intersection="loadMore">
{{ t('logreader', 'Loading older log entries') }}
</IntersectionObserver>
Expand All @@ -35,14 +35,15 @@
</td>
</tr>

<LogTableRow v-for="row, rowNumber in sortedRows"
<LogTableRow v-for="(row, rowNumber) in renderedItems"
:key="rowNumber"
:row="row"
class="log-table__row"
@show-details="showDetailsForRow" />
</tbody>
<tfoot>
<tr v-if="sortedByTime !== 'ascending'">
<td colspan="5" class="log-table__load-more">
<tfoot role="rowgroup" class="log-table__footer">
<tr v-if="sortedByTime !== 'ascending'" class="log-table__load-more">
<td>
<IntersectionObserver v-if="logStore.hasRemainingEntries" @intersection="loadMore">
{{ t('logreader', 'Loading older log entries') }}
</IntersectionObserver>
Expand All @@ -59,17 +60,22 @@
<script setup lang="ts">
import type { ILogEntry, ISortingOptions } from '../../interfaces'

import { computed, nextTick, ref } from 'vue'
import { computed, nextTick, onMounted, onBeforeUnmount, ref } from 'vue'
import { translate as t } from '@nextcloud/l10n'
import { useSettingsStore } from '../../store/settings'
import { useLogStore } from '../../store/logging'
import { debounce } from '../../utils/debounce'
import { logger } from '../../utils/logger'

import IntersectionObserver from '../IntersectionObserver.vue'
import LogDetailsModal from '../LogDetailsModal.vue'
import LogTableHeader from './LogTableHeader.vue'
import LogTableRow from './LogTableRow.vue'
import LogSearch from '../LogSearch.vue'

// Items to render before and after the visible area
const bufferItems = 3

Check warning on line 77 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L77

Added line #L77 was not covered by tests

const settingsStore = useSettingsStore()
const logStore = useLogStore()

Expand Down Expand Up @@ -110,8 +116,9 @@
}

/**
* Reference to the table body, used for keeping scroll position on loading more entries
* Reference to the table elements, used for keeping scroll position on loading more entries
*/
const tableRoot = ref<HTMLElement>()

Check warning on line 121 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L121

Added line #L121 was not covered by tests
const tableBody = ref<HTMLElement>()

/**
Expand Down Expand Up @@ -146,49 +153,131 @@
sorted.sort((a, b) => order(byLevel, sortedByLevel.value, a, b) || order(byApp, sortedByApp.value, a, b) || order(byTime, sortedByTime.value, a, b))
return sorted
})

/**
* Virtual scrolling logic
*/
const resizeObserver = ref<ResizeObserver | null>(null)

Check warning on line 160 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L160

Added line #L160 was not covered by tests

const firstVisibleRowIndex = ref(0)
const startIndex = computed(() => Math.max(0, firstVisibleRowIndex.value - bufferItems))

Check warning on line 163 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L162-L163

Added lines #L162 - L163 were not covered by tests

const tableRootHeight = ref(0)
const tableHeadHeight = ref(44)
const tableRowHeight = ref(42)
const itemsInViewport = computed(() => Math.ceil((tableRootHeight.value - tableHeadHeight.value) / tableRowHeight.value) + bufferItems * 2)

Check warning on line 168 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L165-L168

Added lines #L165 - L168 were not covered by tests

const renderedItems = computed(() => sortedRows.value.slice(startIndex.value, startIndex.value + itemsInViewport.value))

Check warning on line 170 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L170

Added line #L170 was not covered by tests

const tbodyStyle = computed(() => {
const isOverScrolled = startIndex.value + itemsInViewport.value > sortedRows.value.length
const lastIndex = sortedRows.value.length - startIndex.value - itemsInViewport.value
const hiddenAfterItems = Math.min(sortedRows.value.length - startIndex.value, lastIndex)

Check warning on line 175 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L172-L175

Added lines #L172 - L175 were not covered by tests

return {

Check warning on line 177 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L177

Added line #L177 was not covered by tests
paddingTop: `${startIndex.value * tableRowHeight.value}px`,
paddingBottom: isOverScrolled ? 0 : `${hiddenAfterItems * tableRowHeight.value}px`,
}
})

onMounted(() => {
resizeObserver.value = new ResizeObserver(debounce(() => {

Check warning on line 184 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L183-L184

Added lines #L183 - L184 were not covered by tests
tableRootHeight.value = tableRoot.value?.clientHeight ?? 0
tableHeadHeight.value = tableRoot.value?.querySelector('thead.log-table__header')?.clientHeight ?? 44
tableRowHeight.value = tableRoot.value?.querySelector('tr.log-table__row:not(.expanded)')?.clientHeight ?? 42
logger.debug('VirtualList resizeObserver updated')
onScroll()

Check warning on line 189 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L188-L189

Added lines #L188 - L189 were not covered by tests
}, 100))

resizeObserver.value.observe(tableRoot.value!)
tableRoot.value!.addEventListener('scroll', onScroll)

Check warning on line 193 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L192-L193

Added lines #L192 - L193 were not covered by tests
})

onBeforeUnmount(() => {

Check warning on line 196 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L196

Added line #L196 was not covered by tests
if (resizeObserver.value) {
resizeObserver.value.disconnect()

Check warning on line 198 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L198

Added line #L198 was not covered by tests
}
})

/**
* Update the first visible row index on scroll (max 0 to prevent negative index)
*/
function onScroll() {
firstVisibleRowIndex.value = Math.max(0, Math.round(tableRoot.value!.scrollTop / tableRowHeight.value))

Check warning on line 206 in src/components/table/LogTable.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTable.vue#L206

Added line #L206 was not covered by tests
}
</script>

<style lang="scss" scoped>
.log-table {
width: 100%;
height: 100%;
overflow: scroll;
overflow: hidden;

&__table {
width: calc(100% - 12px);
margin-inline: 6px;
table-layout: fixed;

// Necessary for virtual scroll optimized rendering
display: block;
overflow: auto;
height: 100%;
will-change: scroll-position;
}

&__load-more {
text-align: center;
padding-block: 4px;
}
display: flex;

th, td {
// level column
&:nth-child(1) {
width: 108px;
}
// app column
&:nth-child(2) {
width: 168px;
:deep(td) {
flex-basis: 100%;
text-align: center;
padding-block: 4px;
}
// message column
&:nth-child(3) {
width: 418px;
}
// time column
&:nth-child(4) {
width: 168px;
}

&__header,
&__body,
&__footer {
display: flex;
flex-direction: column;
width: 100%;

:deep(tr) {
display: flex;
}
// actions column
&:last-child {
width: 62px; // 44px button + 18px padding

:deep(th),
:deep(td) {
flex-shrink: 0;

// level column
&:nth-child(1) {
width: 108px;
}
// app column
&:nth-child(2) {
width: 168px;
}
// message column
&:nth-child(3) {
width: 418px;
flex-grow: 1;
}
// time column
&:nth-child(4) {
width: 25ch; // "Mar 10, 2025, 12:00:00 PM" length
}
// actions column
&:last-child {
width: 62px; // 44px button + 18px padding
}
}
}

thead {
&__header {
position: sticky;
top: 0;
z-index: 1;
min-height: 44px;

:deep(th) {
Expand All @@ -200,7 +289,7 @@
}
}

tbody {
&__body {
// Some spacing for first row
&:before {
content: '\200c';
Expand All @@ -210,5 +299,8 @@
}
}

&__row {
min-height: 42px;
}
}
</style>
15 changes: 11 additions & 4 deletions src/components/table/LogTableRow.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
<div class="row-message__text">
<LogException v-if="row.exception" :exception="row.exception" />
<!-- Show log message if either there is no exception or a custom message was added -->
<div v-if="!row.exception || row.message !== row.exception.Message" class="row-message__text_message" :title="row.message">
<div v-if="!row.exception || (isExpanded && row.message !== row.exception.Message)"
class="row-message__text_message"
:title="row.message">
{{ row.message }}
</div>
</div>
Expand Down Expand Up @@ -117,6 +119,11 @@
*/
const isExpanded = ref(false)

// FIXME: as components reused, another row will be expanded during scroll, so should close on prop change
watch(() => props.row, () => {
isExpanded.value = false

Check warning on line 124 in src/components/table/LogTableRow.vue

View check run for this annotation

Codecov / codecov/patch

src/components/table/LogTableRow.vue#L123-L124

Added lines #L123 - L124 were not covered by tests
})

/**
* Human readable and localized level name
*/
Expand Down Expand Up @@ -175,10 +182,10 @@

<style lang="scss" scoped>
td {
display: table-cell;
display: block;
overflow: hidden;
text-overflow: ellipsis;
vertical-align: top;
min-height: 42px;
padding-block-start: 4px;
padding-inline: 18px 0;
}
Expand Down Expand Up @@ -215,7 +222,7 @@
}

tr {
display: table-row;
display: flex;
&.expanded {
white-space: normal;

Expand Down
2 changes: 1 addition & 1 deletion src/store/logging.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const mocks = vi.hoisted(() => {
})

vi.mock('@nextcloud/dialogs', () => ({
showError: mocks.showError
showError: mocks.showError,
}))

vi.mock('../utils/logfile.ts', () => {
Expand Down
1 change: 1 addition & 0 deletions src/store/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export const useLogStore = defineStore('logreader-logs', () => {

/**
* Load entries from string
* @param text clipboard text content
*/
async function loadText(text: string) {
// Skip if aborted
Expand Down
Loading