Add allow_sorting Property#126
Conversation
Greptile SummaryAdds an
Confidence Score: 4/5Safe to merge; the feature is additive and defaults to the existing behavior, so no regressions are expected. The two-level default chain in C is correct, the per-column emit is properly formatted, and the JS guard is straightforward. The only gap is that non-sortable header cells give no visual cue to the user — the cursor and appearance stay identical to sortable columns, so a click simply disappears silently. centrallix-os/sys/js/htdrv_table.js — the header-cell rendering loop could apply a cursor hint when allow_sorting is false.
|
| Filename | Overview |
|---|---|
| centrallix/htmlgen/htdrv_table.c | Adds allow_sorting to both httbl_col and httbl_struct, reads it via htrGetBoolean with correct defaulting (table-level defaults to 1, column-level defaults to the table value), and emits it as an integer per-column in the JS init block. |
| centrallix-os/sys/js/htdrv_table.js | Guards the header-click sort path with cols[ly.colnum].allow_sorting; click is silently swallowed when sorting is disabled, with no cursor or visual indicator change for the user. |
| centrallix-doc/Widgets/widgets.xml | Adds documentation entries for the table-level and column-level allow_sorting properties; descriptions are accurate and consistent with the implementation. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User clicks table header cell] --> B{subkind == headercell?}
B -- No --> C[Not a header click]
B -- Yes --> D{allow_sorting == 1?}
D -- No --> E[No-op: click ignored]
D -- Yes --> F[Build new sort order]
F --> G{Column in sort order?}
G -- asc --> H[Toggle to desc]
G -- desc --> I[Toggle to asc]
G -- absent --> J[Prepend as asc]
H & I & J --> K[Invoke OrderObject on osrc]
subgraph C render time
L[Read table allow_sorting default=1] --> M[Per column: read allow_sorting default=table value]
M --> N[Emit allow_sorting as INT in JS init]
end
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
centrallix-os/sys/js/htdrv_table.js:3090
**No visual feedback for non-sortable header cells.** When `allow_sorting` is `0`, clicking the header is silently swallowed, but the column header renders identically to a sortable one — same cursor, same appearance. A user who clicks a disabled header gets no indication that the action is unsupported. Applying `cursor: not-allowed` to the header cell at setup time (in the `for(var i…)` loop around line 2305) when `!t.cols[i].allow_sorting` would make the intent immediately clear.
Reviews (1): Last reviewed commit: "Rename allow_sort to allow_sorting." | Re-trigger Greptile
|
The issue raised by Greptile is a known lack of feedback that's common across Centrallix, and it's fixed in #90, so we'll ignore it here. |
|
This PR is ready for human review. |
This PR adds the
allow_sortingproperty totable-columnwidgets that allow the app designer to disable clicking the header row of a table to sort by that given column. It also adds theallow_sortingproperty to the entire table, where it serves as a default for columns where the property is not specified. This value defaults toyes, which gives the current behavior.This functionality will be very useful for apps where clicking the header row to sort is either not needed, does not make sense, or is just simply very hard to implement correctly.