Repository navigation
Conversation
remko48
added this pull request to stack #15
October 12, 2026 08:33
remko48
force-pushed
the
stack/g4-grid-widgets
branch
2 times, most recently
from
October 12, 2026 11:46
10a5016 to
c682c13
Compare
Adds the building blocks of a grid view: a canvas that places widgets on a 12 column grid with gridstack, the card around each widget, and the header and text widgets. The widget types are loaded from the server. - the grid reflows to 6 and 1 columns on narrower screens, while the stored layout keeps the 12 column positions - in edit mode widgets can be dragged and resized, or moved and resized from the widget menu with the keyboard - colors from widget content are applied only when they are hex values 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
force-pushed
the
stack/g4-grid-widgets
branch
from
October 12, 2026 11:56
c682c13 to
929306d
Compare
remko48
commented
Oct 12, 2026
remko48
left a comment
remko48
left a comment
Member
Author
There was a problem hiding this comment.
Review (Thorough, round 2)
Each finding is an inline comment on the line it is about, marked 🔴 Blocker / 🟡 Concern / 🟢 Minor. This round takes the updated base stack into account (#13, #14 and #16 fixes, including 2ebf8c7, 2da6fa5 and 777c1b0); the diff of this PR itself is unchanged since it was rebased.
What was verified
- Read the full diff against
stack/g3-views-api-v2, and confirmed it is byte-identical to the round-1 patch (the branch was only rebased onto the updated #16). - Checked the backend widget schemas (
lib/Service/GridWidgetTypes.php): all defaults are scalar ornull, and the hex rule matches the one inAGENTS.md. - Confirmed
tis a global in Tables (main.jsplus the baretalready used instore.js). - Inspected the gridstack 13.3.0 package (
column()defaults,gs-min-whandling). - Checked the commit carries Conventional Commit format,
Assisted-by,Signed-off-byand a SPDX header.
Not run
eslint, stylelint, npm run dev/build, any tests, the app itself. The narrow-canvas finding is reasoned, not reproduced.
This was referenced Oct 12, 2026
…azily - on a canvas narrower than 12 columns drag, resize and the move actions are off with a hint, since that reflow is only shown and never stored - widget types load the first time a widget needs them, in one request, instead of on every app start - the column choice and the layout mapping are pure, named functions with component tests, together with the widget defaults, the grid copy and the hex color check - one deep copy for defaults and grids, which also copies reactive data - a header widget's heading sits below the card's own heading - widgets no longer get an editable attribute they do not declare, and the grid baseline variable is used without a fallback Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Remko <remko@conduction.nl>
remko48
commented
Oct 12, 2026
remko48
left a comment
remko48
left a comment
Member
Author
There was a problem hiding this comment.
Review (Thorough, round 3 — code guidelines and styling)
A strict pass on code guidelines and styling. Each finding is an inline comment, all 🟢 Minor. The 8 round-2 comments are still open: the diff is unchanged, so none of them is fixed yet.
CI
Most workflows never start in this fork: their jobs ask for runs-on: ubuntu-latest-low, a runner label that only exists in the Nextcloud org. I ran them locally at 929306dd3 (WSL, Node 24.20 / npm 11.19):
| Workflow | Result |
|---|---|
| Lint eslint | ✅ pass, 0 warnings |
| Lint stylelint | ✅ pass (also green here) |
| Build Javascript | ✅ pass, and the build leaves the working tree clean |
| Cypress Component Tests | ✅ 5/5 specs, headless Chrome 155 |
| REUSE Compliance | ✅ 1399/1399 files |
| AI Policy | ✅ pass (adds the "AI assisted" label) |
| Block fixup and squash commits | ✅ pass |
Not run: PHPUnit (×4), Integration, Static analysis, Rector, Lint php, Lint info.xml and Playwright, which need PHP, Composer or Docker. This PR changes no PHP and no info.xml. Playwright does exercise it, through the loadWidgetTypes() call on every page.
What was verified
- Re-read every file of the diff.
- Measured Tables' conventions to compare against:
data-cycasing, border tokens, comment casing,info.xmlmin-version 33. - Confirmed the arrow icons have no
*Outlinevariant invue-material-design-icons5.3.1, so the outline-icon rule inAGENTS.mddoesn't apply to them.
- the grid constants and pure helpers move to gridLayout.js, widgetRegistry.js only maps widget types to components - move and resize steps are disabled when they cannot apply - a widget card is labelled by its visible heading, and editor placeholders only show while the page is edited - data-cy names are camelCase, the edit outline uses the max-contrast border and the header subtitle no longer lowers contrast with opacity - GridStack and the resize observer are no longer reactive state, so they need no markRaw; the margin is a named constant - one comment style, JSDoc on every method and one way to use icons Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Remko <remko@conduction.nl>
Signed-off-by: Remko <remko@conduction.nl>
3 of 5 tasks
Signed-off-by: Remko <remko@conduction.nl> Assisted-by: ClaudeCode:claude-opus-5-5
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
1 participant
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.
Frontend building blocks to show a grid view: a canvas that places widgets on a grid, the card around each widget, and the first two widget types (header and text). Nothing uses them yet; the grid view page comes in a later PR of this stack.
What changes
gridstackfor the grid layout (drag, resize, reflow). It does the hard part of a dashboard grid; writing that ourselves would be a lot more code to maintain.GridCanvas: 12 columns on a wide screen, 6 on a medium one, 1 on a phone, based on the canvas width. A narrower layout is only how the page is shown; the stored layout always stays the 12-column one. Widgets can therefore only be arranged at full width: on a narrow screen edit mode shows a hint instead.GridWidget: the card with an optional title and, in edit mode, a menu to configure, remove, move and resize the widget, so the grid can be arranged with the keyboard as well as by dragging. In edit mode the card always shows a heading, the widget's title or else its type name.HeaderWidget(heading, subheading, alignment, colors) andTextWidget(paragraphs, split on empty lines). Colors are only applied when they are hex, the same rule as the backend.GET /api/2/views/widget-typesthe first time a widget is shown, with one shared request; the frontend only maps each type to the component that renders it.🖼️ Screenshots
No visible change yet, since nothing renders the canvas until the grid view page PR.
🏁 Checklist
/backport to stableX.X🤖 AI (if applicable)