Skip to content

πŸ“Š Note details tab in the sidebar - #1974

Open
karlitschek wants to merge 1 commit into
mainfrom
feat/noid/note-info
Open

πŸ“Š Note details tab in the sidebar#1974
karlitschek wants to merge 1 commit into
mainfrom
feat/noid/note-info

Conversation

@karlitschek

@karlitschek karlitschek commented Aug 6, 2026

Copy link
Copy Markdown
Member

The note sidebar hosts Sharing and Versions but says nothing about the note itself. This adds a tab of Notes' own β€” Details β€” with the note's category, a reading estimate, its path, and a read-only marker when the note cannot be written.

Everything but the reading estimate is already on the note in the store, so the tab costs one fetch at most. Size, modification date and owner are not repeated here: #1972 put them in the sidebar header, where they describe the note as a whole rather than belonging to one of its tabs.

The reading estimate shapes the design

The note list payload excludes content, so a note that has never been opened has none client-side. Rather than fetching every body up front, the tab pulls the one note it needs, and only once its tab is actually selected β€” opening the sidebar to share a note does not drag its body down with it. Until then the row shows a placeholder, and a body that cannot be read shows a dash rather than disappearing.

NcAppSidebarTab keeps an inactive tab mounted and hides it in CSS, so the tab's content is rendered only while it is the active one. Otherwise the estimate would recount the whole body on every keystroke of someone editing the note with the sidebar open on Sharing.

The body is fetched for one note at a time, and which note that is guards the result: the sidebar can be sent to another note while a fetch is still on its way β€” from a row of the list, or by the list navigating β€” and the answer to the first request must then neither be reported for the second note nor keep its own fetch from starting.

Counting words means ignoring the markup, otherwise # and ** inflate the number. noteStats.js strips the obvious things β€” fenced code, image syntax, link targets while keeping labels, heading, quote and list markers, setext underlines, emphasis β€” and leaves the rest alone. A full parse would be much more code for a number nobody checks to the decimal. Emphasis is matched within a line, both because it cannot span a blank line and so an unclosed marker does not scan the rest of the note looking for a partner.

Deliberately no word or character count

The Text app already shows both for the note open in rich mode, computed from its parsed document rather than from the markdown. The two definitions disagree β€” for **bold** [label](https://example.com) Text counts 10 characters and a markdown-based count sees 37 β€” so a second pair of numbers here would invite the question of which is right. A reading estimate is the one figure Text does not offer.

Fitting in next to the registered tabs

The tab is not one from the Files registry, so it renders outside the loop over the allow-listed ones, with an order that puts it after them: Sharing stays the first and default tab and Versions keeps its place. Its icon is outlined until its tab is active, following what the sharing icon already does, and its rows are inset to the same 8px the Files tabs put their own content at.

resolvedTab() clamps over both kinds of tab now rather than the registered ones alone, so the fallback can land on this tab when it is the only one there is β€” and the "not available right now" empty state is keyed on that same list, so it cannot render alongside a tab that is on screen. Opening the sidebar no longer skips loading the DAV node when no Files tab is registered, since the header and this tab both need it.

"Details" is offered in the note's action menu next to Share and Versions.

Keeping up with the note

The sidebar is opened for one note, but the list keeps navigating. An open sidebar now follows whichever note is being looked at, the way the Files one does, keeping the tab in view. It watches the note id in the route rather than the store's selectedNote, which nothing ever sets.

Within the tab, the category was reactive but the path was not, and needed a full reload. Two separate causes, both required:

  • updateNote() copies the note attributes only. internalPath is not one of them, and neither is readonly, so both were left behind on every update β€” a refetch could not have refreshed them either.
  • The category endpoint answers with the category string alone, so nothing carried the new path back. setCategory() refetches the note now. A refetch that does not arrive is not the category update having failed, so its failure is not reported as one.

Measured on a dev instance: neither fix, the path never updates; the store fix alone, 18.9s (the next background sync); both, 415ms.

Layout

Rows are stacked β€” label above value β€” rather than laid out in two columns. A path is long enough that a two-column row either wraps inside a narrow value column or has to break the rhythm by spanning the full width on its own; stacking every row removes the special case and gives each value the whole sidebar. It also leaves room for translated labels, which run considerably longer than the English ones.

core/css/apps.scss styles dl/dt/dd for prose in a wide content area: 12px of padding on every side, and a fixed 130px label column aligned to its end. In a sidebar that doubled the height of every row and left the labels ragged, so the component resets all three. Worth knowing for anyone else reaching for a description list.

Tests

playwright/e2e/note-sidebar.spec.ts gains cases that open the details tab from the actions menu, fill its icon only while it is active, estimate the reading time from the note body, load a body for a note that has never been opened, load the body of the note the sidebar moves to while another is still on its way, follow the note the list navigates to, and mark the reading time unavailable when the body cannot be loaded. Its existing tab-count case now expects three tabs rather than two.

src/tests/noteStats.spec.js covers noteStats.js β€” every markup rule stripMarkup() implements, including the non-breaking space a task checkbox may contain, plus a body that is absent or not a string and a reading estimate that neither rounds an existing note down to nothing nor misrounds at the boundaries.

src/tests/notesStore.spec.js covers updateNote() propagating internalPath and readonly. Those are unit tests rather than e2e ones on purpose: the path refreshes within seconds in the e2e container whether the fix is there or not, so no browser test can fail for it.

πŸ€– AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@karlitschek
karlitschek requested review from enjeck and silverkszlo and a lite review from Copilot and removed request for enjeck and silverkszlo August 6, 2026 18:32

This comment was marked as off-topic.

@AndyScherzinger AndyScherzinger added 3. to review AI assisted This PR contains AI-assisted commits enhancement New feature or request labels Aug 11, 2026
@enjeck
enjeck requested a lite review from Copilot August 16, 2026 07:57

This comment was marked as outdated.

This comment was marked as resolved.

@AndyScherzinger AndyScherzinger changed the title Feat/noid/note info πŸ“Š Note info Aug 16, 2026
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/note-info branch 4 times, most recently from 67be3df to 651ea2a Compare August 17, 2026 21:00

@jancborchardt jancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed with @AndyScherzinger – if "Details" moves to be the last (rightmost) tab.

Most of the info is already existing in other places, but yeah we will introduce tabs for versions anyway, so good to keep it consistent. :)

@jancborchardt jancborchardt moved this to πŸ—οΈ At engineering in πŸ– Design team Aug 19, 2026
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/note-info branch 2 times, most recently from bea2946 to 5b66fe2 Compare August 24, 2026 16:48
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/note-info branch 2 times, most recently from 23dc96a to 75d0fff Compare August 30, 2026 10:04
@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/NotesService.js 0.00% 1 Missing ⚠️

πŸ“’ Thoughts on this report? Let us know!

@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/note-info branch 2 times, most recently from cb2fd76 to 48f959b Compare August 31, 2026 08:06
@AndyScherzinger
AndyScherzinger force-pushed the feat/noid/note-info branch 3 times, most recently from dfa45a5 to 762bada Compare September 12, 2026 10:57
@silverkszlo

Copy link
Copy Markdown
Contributor

please resolve the merge conflicts now that #1972 has been merged

Comment thread src/components/NoteInfo.vue Outdated
Comment thread src/components/NoteInfo.vue Outdated
Comment thread src/components/NoteInfo.vue Outdated
Comment thread src/components/NoteSidebar.vue
Comment thread src/noteStats.js Outdated
.replaceAll(/^~~~[\s\S]*?^~~~/gm, ' ')
// images, which have no words to keep, then links, which keep their label
.replaceAll(/!\[[^\]]*\]\([^)]*\)/g, ' ')
.replaceAll(/\[([^\]]*)\]\([^)]*\)/g, '$1')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude suggests the following changes here:

// images, which have no words to keep, then links, which keep their label
-		.replaceAll(/!\[[^\]]*\]\([^)]*\)/g, ' ')
-		.replaceAll(/\[([^\]]*)\]\([^)]*\)/g, '$1')
+		.replaceAll(/!\[[^\]\n]*\]\([^)\n]*\)/g, ' ')
+		.replaceAll(/\[([^\]\n]*)\]\([^)\n]*\)/g, '$1')

...to avoid a performance regression. Without the \n, an opening [ with no ] after it makes the engine scan to the end of the note before the match fails. It then resumes at the next [ and scans to the end again β†’ one full scan of the note per stray bracket. A 120 KB note full of them takes ~2.4 s here, and the tab is frozen for all of it. Bounding the class to a line makes each failed attempt cost one line instead of the whole note: same note, ~2 ms.

Nothing that sits on a single line strips differently, so stray brackets are counted as words exactly as before. The one behaviour change is a link or image label deliberately wrapped across two lines: it stops being recognised as a link, so its brackets stay in the text and count as words. That's the same trade the emphasis pattern below already makes.

@silverkszlo silverkszlo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool, sidebar looks good and complete now, thanks! :)

@AndyScherzinger

Copy link
Copy Markdown
Member

Thanks @silverkszlo πŸ™

I am running another review via an agent, let's see if it returns with findings, else I agree - good to go

The sidebar hosts the tabs the Files sidebar contributes but says nothing about
the note itself. This adds a tab of Notes' own with the note's category, a
reading estimate, its path, and a read-only marker when the note cannot be
written. Size, modification date and owner are already in the sidebar header
and are not repeated here.

Most of it is free: everything but the reading estimate is on the note in the
store.

The estimate is not free, and that shapes the design. The note list payload
excludes `content`, so a note that has never been opened has none client-side.
Rather than fetching every body up front, the tab pulls the one note it needs
and only once its tab is actually selected β€” opening the sidebar to share a
note does not drag its body down with it. Until then the row shows a
placeholder, and a body that cannot be read shows a dash rather than
disappearing. fetchNote() only rejects on a missing note and reports everything
else itself, so the content is what says whether the fetch worked.

The body is fetched for one note at a time, and which note that is guards the
result: the sidebar can be sent to another note while a fetch is still on its
way β€” from a row of the list, or by the list navigating β€” and the answer to the
first request must then neither be reported for the second note nor keep its
own fetch from starting.

Estimating a reading time means counting words without the markup, otherwise
'#' and '**' inflate the number. noteStats.js strips the obvious things β€”
fenced code, image syntax, link targets while keeping labels, heading, quote
and list markers, setext underlines, emphasis β€” and leaves the rest alone. A
full parse would be much more code for a number nobody checks to the decimal.
Emphasis is matched within a line, both because it cannot span a blank line and
so an unclosed marker does not scan the rest of the note for a partner.

Deliberately no word or character count. The Text app shows both for the note
open in rich mode, computed from its parsed document rather than from the
markdown, so a second pair of numbers here would differ from those and invite
the question of which is right. A reading estimate is the one figure Text does
not offer.

The tab is not one from the Files registry, so it renders outside the loop over
the allow-listed tabs, with an order that puts it after them: Sharing stays the
first and default tab. Its icon is outlined until its tab is active, following
what the sharing icon already does, and its rows are inset to the same 8px the
Files tabs put their own content at. resolvedTab() clamps over both kinds of
tab now, so the fallback can land on this one when it is the only tab there is.
"Details" is offered in the note's action menu next to Share and Versions.

The sidebar also follows the note the list navigates to. It was opened for one
note and then stayed on it, so picking another note left it showing the
previous note's sharing, versions and details, which reads as stale data rather
than as a sidebar that belongs to a different note. It now watches the note id
in the route and re-opens itself for whatever the list navigates to, keeping
the tab that is in view, the way the Files sidebar does. Note selection is read
from the route rather than from the store: setSelectedNote() is never called
anywhere, so store.notes.selectedNote is always null. Reusing onSidebarOpen()
means the switch goes through the same path as opening the sidebar in the first
place, so the DAV node, the parent folder and the note body are all reloaded
for the new note rather than half-updated. Opening no longer skips the node
context when no Files tab is registered, since the Details tab needs it.

Two of the values the tab shows would otherwise go stale. updateNote() copies
the note attributes only, so internalPath and readonly β€” reported by the server
but not among them β€” were never refreshed. And the category endpoint answers
with the category string alone, so nothing carried the new path back after a
move; setCategory() refetches the note now, and a refetch that does not arrive
is not reported as the category update having failed.

Assisted-by: Claude Code:claude-opus-5[1m]
Assisted-by: Claude Code:claude-fable-5
Co-Authored-By: Andy Scherzinger <info@andy-scherzinger.de>
Signed-off-by: Frank Karlitschek <frank.karlitschek@nextcloud.com>
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
@AndyScherzinger

Copy link
Copy Markdown
Member

@silverkszlo I am done and CI is green πŸš€

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review AI assisted This PR contains AI-assisted commits enhancement New feature or request

Projects

Status: πŸ—οΈ At engineering

Development

Successfully merging this pull request may close these issues.

5 participants