Repository navigation
Conversation
Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>
An application (context) gets a menu of items that each open a view, a table or a link. Items are loaded with the context in one chunked query, and an item whose view or table no longer exists is reported as broken, computed in SQL. Deleting an application deletes its menu. - a menu item's technical name is unique within its application, kept so by a unique index on (context_id, technical_name) - the item target type is typed as view, table or url in the API - the mapper groups the joined context rows in one place instead of three Co-authored-by: Ruben van der Linde <ruben@conduction.nl> Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Remko <remko@conduction.nl>
remko48
added this pull request to stack #24
October 12, 2026 11:39
2 of 5 tasks
rjzondervan
reviewed
Oct 12, 2026
Views come from more source (owned, and shared) and there was a race condition that would reset the list of views instead of extending it. Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de> Assisted-by: ClaudeCode:claude-opus-5
…nu items - MenuItemMapperTest runs against a real database: items pointing at a deleted view or table come back broken and links never do, items are ordered and grouped per application, and ContextMapper::findById() and findAll() carry the menu - MenuItemTest serializes each of the three target types - technicalName is a string in the entity and the API: the column is not null Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Remko <remko@conduction.nl>
build(release): v2.3.2
…les-views fix(sidebar): keep views the table listing does not carry
Signed-off-by: Rello <github@scherello.de> Signed-off-by: Rello <Rello@users.noreply.github.com> AI-assistant: Codex (GPT-5) Co-Authored-By: Andy Scherzinger <info@andy-scherzinger.de> Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Folds the two rounds of review fixes: the card source handling in ApiTablesController and the settings dialog, then the scheme and migration paths in ViewService, TablesMigrator and the context page, with the migrator test that covers them. Signed-off-by: Rello <github@scherello.de> Co-Authored-By: Andy Scherzinger <info@andy-scherzinger.de> Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>
Use fn () instead of fn() in ViewController and TablesMigratorTest, as php-cs-fixer requires. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
View creation silently coerced any unsupported layout to the table layout and still reported success, while updates reject the same values in ViewUpdateInput::normalizeLayout(). Reuse that normalizer on create so a typo returns a bad request instead of persisting a different layout than the caller asked for. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Importing a table or context scheme remapped columnSettings, sort and filter through the uuid map but persisted the exported card source column ids unchanged, so tiles/gallery views could reference a nonexistent column or, after an id collision, show data from an unrelated column of the target table. Resolve both card sources through the columnId/columnUuid pairs the exported column settings already carry, and drop a source that cannot be resolved rather than keeping a stale id. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
PublicElement bound a localViewSetting that was never declared and passed no view setting to TableView, so a view saved as tiles or gallery always fell back to the table layout when opened through a public link. Initialise the setting from the shared view and pass it down to the renderer. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The named actions slot was only rendered inside the table header, which is skipped for tiles and gallery, so opening a card layout hid the whole action menu including the view settings entry used to switch the layout back. Render the slot above the card grid when a card layout is active. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The migration was still named Version1000Date20260318000000, a prefix far below the migrations added to the repository since this branch started, so it sorted ahead of everything already released. Rename it to the 2.4.0 target version so the ordering reflects the release it ships in. The step guards every change with hasTable()/hasColumn(), so re-running it under the new name on an installation that already applied the old one is a no-op. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
ViewService::update() validated the stored cardBackgroundSource and cardTitleSource on every request, so removing a column from a view failed with 400 even when the request never mentioned viewSettings. Callers were forced to resend the card settings just to make an unrelated column change succeed. Validate strictly only when the request actually carries view settings, and otherwise drop a card source the new column set no longer contains. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The updated() hook assigned pageNumber on every re-render, and its totalPages === 1 branch fired on each update regardless of whether anything changed. Mutating reactive state from that hook risks feedback loops. Move the clamp into a totalPages watcher so it only runs when the page count actually changes, and keep the number at 1 or above when a filter empties the table. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
CustomTable carried its own copy of the pagination controls, which duplicated PaginationBlock and left the two controls out of sync: changing the page in the footer never reached the block in the toolbar, because only the toolbar copy emitted tables:pagination-changed. Render PaginationBlock in the footer instead and let it adopt pagination changes from the event bus, guarded so an adopted change is not echoed back. This drops the duplicated markup, its icon and component imports and the now unused allPageNumbersArray computed. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The existing component test mounts a three row fixture, so the footer pagination never rendered and the two pagination controls were never exercised together. Mount NcTable with 150 generated rows instead and assert that paging from either control moves the other, that the footer only appears beyond the first page, and that it renders in the gallery layout. Removing the event bus subscription from PaginationBlock fails the two sync assertions, so they guard the behaviour rather than the markup. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The date part still read 20260318, which predates the file itself: it was added to the branch on 2026-05-11. Set it to the day the migration ships so the name orders correctly against the other 2.4.0 migrations. The class name is what gets recorded in oc_migrations, and it already changed with the version prefix rename, so this carries no additional upgrade cost. The step guards every change with hasTable()/hasColumn() and is a no-op where it has already run. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The layout migration targets 2.4.0, but the app still declared 2.3.1-rc.1, which is lower than the version development instances already have installed. Nextcloud refuses to load an app whose declared version is below the installed one, so the branch could not be deployed for testing. Bumps appinfo/info.xml along with package.json and package-lock.json, which this repository keeps in sync. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Cards are grid items and stretch to the tallest card in their row, but the card was a block holding a fixed height image and a body of natural height, so every card carrying less metadata than its neighbours ended in dead space: 22px for two short values, 76px for a single one. Make the card a flex column so the body absorbs the leftover height. The title banner had no height limit. Absolutely positioned against an image wrapper pinned to aspect-ratio 3 / 2, a long title grew past the top of the image and the card's overflow sliced the first line away. It now gets a line budget derived from the rendered card size rather than a fixed count, kept in step by a ResizeObserver on the card container. The observer watches the title as well, because the container keeps its size when only the text size changes, which would otherwise leave the budget stale at 200%. MIN_TITLE_LINES makes the share a preference rather than a hard cap, so an enlarged font shortens the title instead of dropping it to a single line. The clamp sits on an inner span: clamping the banner itself still painted a partial line into its padding. Cards without a background column are not covering an image, so their title may use the whole card. A tile then reads like a note and starts at the top, while a gallery card keeps its title next to the metadata below it. Finally the scrim is darkened from 0.4 to 0.55. White text over a white image region measured 2.85:1, below the 4.5:1 WCAG AA needs at this size and weight, and compliance otherwise depended on which picture a user happened to pick. 0.55 puts the worst case at 4.76:1 in both colour schemes. The accompanying text shadow only firms up the glyph edges where the title crosses a bright part of the image; contrast ratios ignore shadows, so it carries no part of AA. A file that cannot be read made the browser paint the alt text into the card, where it ran into the banner already showing the same words. The image keeps its alt attribute, and with it its accessible name, but the visual fallback is now suppressed so only the broken image indicator remains. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
A view shared by link always came out as a table. The renderer takes the layout from the view setting it is handed, but nothing ever put one there: the public page ships its node through an allowlist of six keys, and neither layout nor viewSettings was among them, so the browser never learned that the view had been saved as tiles or a gallery. Let both keys through and carry them into the element the public page builds. A shared table has no layout of its own and keeps rendering as a table. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
A view embedded in a text or markdown file rendered as a table whatever layout it had been saved with. The widget mounts the same table component as the app, so it only lacked the setting: the reference payload carried no layout, and the widget started from an empty view setting that nothing filled in. Send layout and viewSettings along with a view reference, and seed the widget from them. A table reference carries neither and keeps the table layout. Covered by component tests for both card layouts and for a reference without one. Splitting the mount helper keeps the row tests aliasing a table while the card cases mount without assuming one. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
A preview is served against the viewer's session, so on a public link there is none to authenticate with and the image can never arrive. Cards still asked for one, leaving a broken image on every card of a shared tiles or gallery view. Ask for no preview at all when there is no session. The card then has no background to show and falls into the layout a view already uses when no image column is configured, so the shared card needs no styling of its own. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
layout and viewSettings travel in the same viewSetting object as the local tweaks, so emitting an empty object dropped them and a tiles or gallery view fell back to the table rendering until the next reload. Only the keys the reset button reports on are cleared now. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
updateView() rewrote both card sources on every save, so renaming a plain table view stored a background pointing at the first column, which is rarely an image. Together with a non clearable select that made "no background image" unreachable even though the model, the API and the rendering all treat null as a valid state. Sources are now only written once picked, and the background can be cleared again. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
An empty candidate list skipped validation altogether, so a column id from an unrelated table was accepted whenever the view had no columns of its own. With no candidates every non null source is now invalid, which is what the rest of the write path already assumes. The docblock describing the assertion also moves back onto it. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
getViewSettingsObject() parsed the stored column through the strict input factory, so a single unparsable view_settings row made jsonSerialize() throw and took down every endpoint that lists that view. Reading now drops a value it cannot use, matching how the layout column is already normalised on read, while writes keep rejecting bad input. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
createViewSettingsFromInputData(), createImportedViewSettings() and remapViewCardSources() each carried a fallback for top level cardBackgroundSource and cardTitleSource keys. That shape was never released, so no stored data uses it and the branches were unreachable. The migrator test covering it now asserts that a view without settings passes through untouched. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The ready made URL branch accepted any path ending in /core/preview and forwarded the query string verbatim, so a cell author chose part of the path and all parameters of an authenticated same origin request. The /f/<id> match also ignored the host, contradicting the comment above it: a link naming a foreign server resolved to a local file of the same id. Both spellings now go through one helper that refuses anything cross origin and rebuilds the preview URL from the extracted id, so neither the path nor the query can be influenced. The requested size becomes a named constant. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
dropOrphanedCardSources() ran on every update that did not carry viewSettings, so a rename, an emoji or a sidebar reorder rewrote the card sources and, for a view without an explicit column selection, paid for a findAllByManagedView() query to do it. Only a changed column selection can orphan a source, and deleting a column is already handled on its own path. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
tables:pagination-changed carried no node identity, so once PaginationBlock subscribed to it as well every pagination control on the page followed every other. A context renders many tables and views at once, where paging one paged them all. The payload now names its element and receivers ignore anything else. The guard against echoing an adopted value no longer depends on watcher flush order either: the adopted pair is remembered and compared instead of clearing a flag on the next tick. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The observer callback wrote the line count that resizes the title it was observing, which made the browser report an undelivered notification loop. Measuring on the next frame lets the layout settle first. The observed title also belonged to a card that paging replaces, leaving the observer on a detached element, so the layout is observed again whenever the page changes. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The layout request was added after the column and row ones, so inserting a view as content waited for three round trips in sequence. None of them depends on another. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
View::getLayoutNormalized(), ViewUpdateInput::normalizeLayout() and importView() each spelled out the set of layouts, and the frontend repeated the card layouts in two components. A backing enum and matching frontend constants now hold the values once. importView() keeps accepting a layout it cannot resolve by dropping it, so a scheme written by another version still imports, but it says so rather than leaving the leniency implicit in a second copy of the list. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The template called getPreviewUrl twice per row, once in the v-if and once in the src binding, each parsing the cell as JSON and looking its column up again, while the title and metadata helpers repeated the same column lookups for every row. A page of a hundred cards paid for all of it on each render. The values are now derived in one computed pass over the page, with the title and background columns resolved once for the whole set. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
getDisplayValue() guarded against a column without getValueString, but every column reaching the card layouts is a columnClass instance, so the branch never ran. The layout picker wrapped a single radio row in a presentation table, which is a layout table for something flex does, and its title rule matched no element. The fallback to the second column as a card title now says why it is the second one. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
layout and viewSettings were copied into viewSetting, the object holding a viewer's unsaved tweaks, so a manage gated property of the view and a local adjustment became indistinguishable. That is what let "Reset local adjustments" drop the layout. Both now reach the table as their own props, sourced from the view itself, and viewSetting.layout means only "this viewer switched layout". The rendered layout is that override falling back to the saved one, so resetting returns to the view's layout instead of erasing it, and the reset button appears once a viewer has switched. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Only a manager can open the view settings, so a recipient of a shared view had no way to read it as anything but the layout it was saved with. The toolbar gains a menu button at its trailing edge listing the three layouts, each with its own icon. Choosing one writes the local view setting, so it applies at once, is never persisted, and is cleared again by "Reset local adjustments" or by leaving the view. The trigger carries the icon of the current layout and is labelled "Switch layout", since an icon-only button would otherwise have no accessible name. It appears only where the view was set up for cards, meaning a card title source is configured or the saved layout is already tiles or gallery: cards of a view without a title source would carry nothing but row numbers. That is a presentation choice, not a constraint, so nothing on the write path enforces it. An embedded widget declines the switch outright, being a snapshot of a view inside someone else's document, and the smart picker preview inherits that; public links and contexts keep it. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
A card was a <button> holding a div, a list and, wherever a rich text column carries one, a link. A button may contain none of those: it takes phrasing content with no interactive descendant, and the parser does not repair the tree, so the invalid markup reached the DOM. A link inside a card also bubbled its click to the button, which followed the link and opened the row editor at the same time. The card is now a div with the button role, keyboard activation on enter and space, and a focus ring of its own. Clicks coming from a link are left to the link. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Card sources drop to null when they no longer resolve, which contradicted the soft-invalidation rule as written. The rule now says why a reference that only selects how a row is displayed is treated differently from one that decides which rows or values are returned: an id that fails to remap on import may address a live column of an unrelated table, and a view without card sources falls back to the text-only card layout, so there is no broken state for a user to repair. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
notifyNodeUpdate() and the share payload carried only title and emoji, so a federated gallery view rendered as a table on the receiving instance with nothing to say the layout had been lost. The layout travels with both, and a value the receiving version does not know is ignored rather than stored. The card sources stay behind on purpose: they are column ids of the sending instance and would address unrelated columns after the hop, so a federated card layout renders text-only. Adding them later means carrying column uuids, which the payload can gain without breaking a receiver, since unknown keys are ignored. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Opening an application by its URL, rather than by clicking it in the navigation, left the page blank: setContext() looked the context up with findIndex() and assigned straight to that index, so on the first load -- when the list has not been fetched yet and the lookup returns -1 -- contexts[-1] named a property instead of appending. The getters iterate the array, so activeContext stayed undefined and every branch of the template was false. Reading the icon off it threw first, leaving "An error occurred while loading the application" instead. Append when the context is not in the list yet, as the store convention for findIndex lookups requires, and drop the second copy of the icon assignment. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The card layout and the link cell each grew their own way of reading a file id out of a link and building a preview request, and the two disagreed. The link cell matched a lowercase fileid only, which is not the spelling the server writes, so a preview URL never resolved through it; it also read an id out of a link to another host and then requested that id from this one. The card layout, in turn, anchored the id to the end of the path and did not know the openfile parameter that a Files deep link carries. Move the matcher and the URL builder to a shared helper that takes what both need: the link is parsed rather than pattern matched, a foreign origin is refused, fileid and openfile are read whatever their case, and the id may be followed by more path. Each caller keeps what is its own -- the link cell only previews a files reference, while a card background may be any column and so probes the fields a stored link may hold -- and both now prefer an id the picker recorded outright. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>
…te related tests Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>
…istent-layout-modes-in-nextcloud-tables 🖼️ Add persistent per-view layout modes (table / tiles / gallery)
A menu item can target a page of the application it belongs to. It is reported broken when that page is gone or belongs to another application. Assisted-by: ClaudeCode:claude-opus-5-5
This was referenced Oct 15, 2026
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
7 participants
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.
Applications (contexts) get a menu: an ordered list of items that each open a view, a table or a web link. This PR adds the data model and returns the menu with the application; saving it comes in the next PR.
What changes
tables_contexts_menu_item(label, icon, target type + id or url, technical name, order). Every item has a technical name, unique within one application, enforced by a unique index.MenuItementity +MenuItemMapper: the menus of all requested applications are loaded in one query (chunked for Oracle), not one per application.broken: true, worked out in the same query, so the UI can show it and the owner can fix it.ContextMapper: the three copies of the "group joined rows per context" code are replaced by oneformatResultSet(). The navigation bar query skips loading menus, since it doesn't show them; theremenuItemsis always empty.menuItemsis added to the context response (OpenAPI and TS types regenerated).Page target
Per the reply on nextcloud#3075, a menu item can also open a page of its application (
targetType: 'page',targetId= id intables_contexts_page). The menu query left-joins the page on id and application. An item isbrokenwhen its page is gone or belongs to another application, like a deleted view or table.Tests:
MenuItemTest(page in the target-type provider),MenuItemMapperTest::testItemsWhosePageIsGoneOrForeignAreBroken(live page, missing page, page of another application).OK (8 tests)/OK (5 tests), psalm and cs:check clean, OpenAPI regenerated. Prepared in the Conduction fork for porting upstream by @remko48. Prepared with Claude Code.🖼️ Screenshots
Backend only, no UI changes.
🏁 Checklist
/backport to stableX.X🤖 AI (if applicable)