🎐 Zen mode - #1970
Open
karlitschek wants to merge 10 commits into
Open
Conversation
karlitschek
requested review from
AndyScherzinger and
silverkszlo
and
a lite review from Copilot
and removed request for
enjeck and
silverkszlo
August 6, 2026 14:18
AndyScherzinger
force-pushed
the
feat/noid/zen-mode
branch
from
August 9, 2026 18:29
7db52c7 to
a67db34
Compare
AndyScherzinger
force-pushed
the
feat/noid/zen-mode
branch
from
August 9, 2026 18:45
896be94 to
5054470
Compare
enjeck
reviewed
Aug 9, 2026
enjeck
left a comment
enjeck
left a comment
Contributor
There was a problem hiding this comment.
Haven't properly reviewed yet. Would like to see a screenshot or vid of what it looks like first.
From a brief look, I do see there are too many comments in the code. We don't need a comment to justify every choice IMO. Makes the code look cluttered and easy for some comments to get stale/outdated and bring more confusion in the future.
Contributor
|
Maybe we should add a line in our AGENTS.md's code style to avoid adding too many comments? I notice that whenever I prompt the AI models to do a certain change and explain why, they overdo it and add comments to the code explaining their changes. Even when its not necessary and the code would be more readable/understandable without it. It's a fine balance. Maybe this only bothers me and everyone else is fine with the many comments, haha 🤷♀️ |
Member
I think is is a general point and valid, also seen it elsewhere and I agree, comment should explain the behavior/function etc. but not document decision that might fade or change over time and then the comment is confusing because it did not get updated. I'll make a PR in parallel, to tackle that 👍 |
Writing anything longer than a shopping list means looking past a category sidebar and a list of every other note. Zen mode removes both and centres the note, leaving the browser and the Nextcloud header where they are. This is not the existing "Full screen" action, and it does not replace it. That one calls the browser Fullscreen API: the whole display becomes the note, which is great for a projector and disruptive if you still want your tabs, your clock and your notifications. Zen mode stays inside the window and only removes the app's own chrome. The two compose — full screen while in zen mode gives just the text on the display. Implementation notes: * The state lives in the Pinia app store, deliberately client-side. It is something you switch on to write a paragraph and off again, so it has no business in the settings the mobile clients read, and it does not survive a reload. * The layout rules live in an unscoped style block in App.vue, because the navigation and the note-list pane belong to @nextcloud/vue and a scoped style cannot reach them. The class goes on NcContent, whose single root (#content-vue) is an ancestor of both. * Centring the editor is unconditional in zen mode. Outside it, that only happens above 1600px, where there is room for the list beside the note. Two guards, because zen mode hides the navigation while the toggle sits inside the editor: * Escape leaves zen mode, unless the conflict dialog is open and wants the key for itself. * If the note stops being editable — deleted in another session, or a read error — zen mode switches itself off, since the toggle disappears with the editor and there would be nothing left to click. Unmounting resets it for the same reason. The periodic note refresh does not touch `loading`, so it cannot trip this. Ctrl + . toggles it, shown in the action's tooltip next to the existing Ctrl + / for preview. Only the plain and preview editors get the toggle; rich mode hands the editor to the Text app, which brings its own chrome. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com> Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The toggle was an action inside NotePlain, which only renders when noteMode is 'edit' or 'preview'. SettingsService::getAvailableEditorModes() puts 'rich' first and getListAttrs() takes $values[0] as the default, so on any instance with the Text app enabled — the default — the entry was never rendered at all. It was invisible for exactly the people most likely to look for it. Copying the action into NoteRich would not have worked either: NotePlain's .action-buttons are fixed at top/inline-end, and the Text app's menubar is `position: sticky; top: 0; width: 100%` with its own controls pushed to the same corner, so the two would have overlapped. Zen mode hides the app's navigation and note list — it is a property of the app shell, not of one editor — so the control now lives in the shell: * "Zen mode" sits in the navigation footer next to "Notes settings", which is rendered for every editor mode. One implementation, no per-editor duplication, and nothing to collide with. * Leaving is a button in the opposite corner, shown only while zen mode is on. Bottom corner on purpose: the top belongs to NotePlain's action menu and to the Text menubar. * Ctrl + . and Escape moved to App.vue with the state they act on. Both bail out while a dialog is open so it keeps Escape for closing itself, which also replaces NotePlain's narrower conflict-dialog check. Because the exit button is always visible while zen mode is on, the guards that turned zen mode off when the note stopped being editable are gone: they existed only so the mode could not strand a user with no way back, and there is now always a way back. NotePlain is left untouched by this feature. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com> Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
It sat in the opposite corner from the "Zen mode" entry in the navigation footer, so entering and leaving were gestures at opposite ends of the window. Putting the exit where the entry just was makes it the same click in the same place. Still the bottom corner rather than the top: NotePlain's action menu and the Text app's sticky menubar both live up there. Nothing in either editor is anchored to the bottom — EasyMDE is configured with `toolbar: false` and `status: false` — so the corner is free. inset-inline-start rather than left, so it follows the reading direction in RTL locales. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com> Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…rner
The button was landing at the middle of the inline-end edge, squeezed and
barely visible, instead of the bottom corner it was styled for.
`position: fixed` was never applied. NcButton's own rule is
`.button-vue[data-v-…] { position: relative }` — an attribute selector, so
specificity (0,2,0) — while `.zen-exit` on the button was a single class at
(0,1,0) and lost, whatever the source order. The button therefore stayed
`relative`, became an ordinary flex item of #content-vue's row, and got
pushed to the end of that row and centred vertically.
Wrapping it in a plain div fixes it at the root: nothing in the bundle styles
that div, so the rule is unopposed. Confirmed against the compiled CSS rather
than assumed this time.
Also raised the resting opacity from 0.6 to 0.7 and moved the hover rule to
:focus-within, since the wrapper is no longer the focusable element. It is the
only visible way out of zen mode, so it should be subdued rather than nearly
invisible.
Assisted-by: Claude Code:claude-opus-5[1m]
Signed-off-by: Frank Karlitschek <karlitschek@users.noreply.github.com>
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Addresses review feedback on the zen mode shortcut handling: - The shortcut hint was hardcoded to "CTRL + ." while the handler also accepts metaKey, so macOS users were told to press a key that is not the one that works. The hint is now derived from the platform and reads "Cmd + ." on Apple devices. It was hardcoded in two places, the navigation entry and the floating exit button, and both now share one computed value. - Matching the toggle on event.key alone made it layout-dependent, as the character a key produces varies by locale. Matching on event.code === 'Period' pins it to the physical key instead, with the event.key check kept alongside so layouts that place a period on another key, and the numpad decimal, keep working. - The open-dialog guard ran a document.querySelector on every keydown, including ordinary typing. The handler now identifies the relevant combos first and only touches the DOM once a key is one it handles. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Exercises the feature through the UI against the bundled e2e server: entering from the navigation footer hides the navigation and the note list, the exit button and Ctrl/Cmd + . both toggle back, Escape only leaves, the shortcut is ignored while a dialog is open, the note stays editable, and the mode does not survive a reload. The shortcut hint is asserted against the browser's own platform so the test holds on macOS and Linux alike. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The entry and the shortcut were available on every route. On the welcome screen zen mode hid the navigation and the note list, leaving nothing but the exit button on an otherwise empty page. Gate both on `canUseZenMode`, and watch it so the mode turns itself off when the last note is deleted and the app routes back to the welcome screen. Covered by a Playwright test that clears the notes through the API to reach a genuine empty state, since the app routes straight to the first note whenever one exists. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Twenty-eight of the added lines were comments justifying individual choices, which is noise in the reading and goes stale as the code moves. The reasoning already lives in the commit messages. Keeps one short line where the code is genuinely surprising: the wrapper div around NcButton, the unscoped style block, the splitpanes width override, the corner the exit button sits in, and the `code`/`key` pair in the shortcut check. Applies the store comment as suggested in review. Drops the non-ASCII characters those comments carried along. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
NcAppNavigationItem renders `title || name`, so the tooltip on the "Zen mode" entry showed nothing but "Ctrl + ." - the shortcut replaced the label it was meant to annotate. Both controls now spell out the action and the shortcut together. The key names are no longer sent to Transifex on their own: translators get one sentence with a placeholder instead of two keyboard glyphs. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The platform pattern and the open-dialog selector were spelled out where they were used. Naming them says what they are without a comment. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
AndyScherzinger
force-pushed
the
feat/noid/zen-mode
branch
from
August 9, 2026 19:22
5054470 to
60b4865
Compare
1 task
AndyScherzinger
approved these changes
Aug 9, 2026
AndyScherzinger
left a comment
AndyScherzinger
left a comment
Member
There was a problem hiding this comment.
Tested successfully on latest Firefox, added some polishing (via AI)
tomek2k1
pushed a commit
to Dataport/nextcloud-notes
that referenced
this pull request
Aug 11, 2026
Comments must document the code itself (signatures, behavior, constraints) — never progress, decisions, or changes, which belong in the commit message or PR discussion and go stale in code. Comments must stay brief and match the surrounding comment density. Follow-up to the discussion in nextcloud#1970. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(please ignore the profiler bottom bar...)
🤖 AI (if applicable)