Make the cascade the source of truth for styled properties - #64
Conversation
A View binds `padding` and `padding-left` to separate Inlets over the same bytes, and View::bind applies them in Inlet declaration order. Whichever is declared later therefore wins, regardless of which Selector the cascade preferred: a class rule setting `min-height` loses to a type rule setting `min-size`, because `min-height` is declared first. Attributes are now stored as longhands only. A shorthand is split on the way in, so the cascade resolves each longhand independently, as CSS does, and declaration order stops deciding the outcome. Both entry points expand: the parser's addAttribute, and addAttributes, which now merges per attribute so a shorthand arriving from JSON or from another Style is treated the same as one from a stylesheet. A single Number still splats to every longhand, matching the rectangle binding, so `padding: 2` keeps meaning all four sides. A value of any other shape is stored unexpanded and left for the Inlet binding to reject, rather than being dropped here.
`frame` named the same bytes as `left`, `top`, `width` and `height`, which is exactly the aliasing the previous commit set out to remove: a Style holding both left an Inlet's declaration order, not the cascade, deciding which won. No stylesheet or view definition in this tree or in Quetoo has ever used it. It remains bindable from JSON, where a View definition names a frame directly rather than cascading.
`hidden` was both a CSS property and a runtime one: View::setHidden writes it 70 times across this tree and Quetoo, and four stylesheet rules write it too, so whichever ran last won. That is the same friction `left` and `top` had against the frame, and it is the one property that would break once a Style resolves its Inlets totally, since an owner's setHidden would be undone by any restyle that says nothing about visibility. Styling now sets View::visibility, an unspecified/visible/hidden tri-state, and setHidden sets View::hiddenByOwner. View::hidden is derived from the two: styling wins where it says anything, the owner wins where it does not. Readers are unchanged. The precedence is what the existing uses already assume. Panel hides its accessoryView in code while a Quetoo stylesheet shows it for dialogs, which works today only because a Style writes the properties it mentions and no others. Checkbox's check mark and CollectionItemView's selection overlay are driven purely by :selected rules, and no code touches them. The CSS property is renamed to `visibility` to say which of the two it is. Two direct writes to the derived field, in Panel and ScrollView, now go through setHidden.
`.options > Option` replaced the whole padding rather than widening it, so an Option lost the 2px above and below that `Option` gives it the moment the flyout opened. Select sizes itself from its Options, so the control shrank by 4px on open and grew back on close.
styledSize was a second, weaker minSize. View::sizeThatFits floored its children-derived sum against it, and then clamped that against minSize a few lines later: the same floor, twice, except that styledSize only applied on the container branch and had to be memoized from the frame on every applyStyle to work at all. A container that needs to keep a size its content cannot give it now says so with `min-width` or `min-height`, which is what it meant. Only two rules here relied on the old behaviour, Slider and TextView, and both are one word longer for it. `width` and `height` still set the frame directly, which suits the views that use them: an icon sized by its style, not by its content.
Twelve properties were assigned in initWithFrame on Views a Selector already reaches, several of them onto rules that existed and simply lacked the declaration. Setting them from C means a stylesheet cannot override them, and it is the C half of a split source of truth: the same property is authored in two places depending on the widget. They are now declarations in Assets/stylesheet.css, which Theme::init always installs, and the assignments are gone. Two new rules name Views that had none, CollectionItemView's ImageView and Text; the rest extend a rule already there. This also removes twelve of the assignments that a total Style resolution would reset, since an authored value that lives in the cascade is never absent from it.
`left` and `top` position a View on an axis that layout leaves alone, so an axis that also has an alignment overwrites them on the next pass, silently. Authoring both is a mistake with no feedback: the rule parses, binds, and is then discarded. View::applyStyle now warns, per axis, so that an internally aligned View placed by `left` -- which is what Box does with its label -- stays quiet, having neither axis' bits set. Raised as WarningTypeStyle, which View::applyThemeIfNeeded already clears before each application, so it tracks the current Style rather than accumulating. Nothing in this tree or in Quetoo trips it today.
…to it The values left in C were not class defaults and had no Selector to move to: a colour picker's swatch takes the colour being picked, an Input's alignments follow its orientation, and a ViewController's root View is a plain View no rule can name. They now go into each View's own Style, which is where an instance's authored values belong and, being merged last, is the one layer a Style resolution can never overwrite. That did not work before. View::applyTheme skips applying when the same Selectors still match, and a change to View::style changes no Selector, so the element level was in practice write-once: the picker's swatch kept its first colour. View::invalidateStyle now marks the View as needing its Style applied whatever the Selectors say, which is what callers were already asking for by invalidating.
A Style that stopped matching went on applying: View::applyStyle wrote the properties the cascade provided and left the rest holding whatever the last Style had set. A closed Select kept the padding its open flyout had. The defaults are declarations now. The universal selector carries View's, at a specificity below every other rule, and a type selector carries each subclass's: a deeper type outranks its superclass, so Select's `selection: single` beats Control's `selection: none` for a Select. Every application therefore starts from the same place and the cascade decides the whole result. Nothing may assign a styled property in C any more, since the cascade overwrites it. Input's axis, Select's selection and ScrollView::setScrollBarVisibility each did, and each now goes through a Style; Input's axis is what put every Checkbox and Slider under its label. The frame is not among them. `left`, `top`, `width` and `height` share it with layout and with any owner that positions its own subviews, so they are bound only when a Style provides them.
Both defaults had to be written in C because neither was zero, which is the whole reason View::initWithFrame still knew anything about styling. A maxSize of zero now means unbounded, per axis, through ViewClampSize. A View capped at nothing is not a View anyone wants; hiding it is what they meant. `max-width: 460` alone therefore leaves the height free, as it reads. pointer-events becomes auto or none, as it is in CSS, rather than a bool whose default was true. `pointer-events: none` says what it does, where `pointer-events: false` had to be read twice. Nothing outside this stylesheet used either spelling.
The angle was measured clockwise from north, which made `180` the sensible default and left View::initWithFrame writing one styled property in C. Measuring from south puts `0` at the same place, and View::initWithFrame now sets no styled property at all: the stylesheet holds every default. Only the vertical axis moves. `90` is still to the right and `270` still to the left, so the gradients in Quetoo's chrome HUD, which are all one or the other, are unaffected.
Two things the stylesheet defaults broke, both from code reading or writing a styled property outside the cascade. Select::addOption selects the first option when nothing is selected yet, and asks Control::selection whether it may. Options are added as a Select is built, before any Theme reaches it, so the answer was the zero value and no option was ever selected until the user picked one. It sets its own selection again at construction; the stylesheet keeps it through a restyle. The HUD example built its whole interface in C, assigning alignment, autoresizing-mask, axis and spacing directly, and the universal selector now overwrites all four. It sets them on each View's own Style instead, which is what a consumer with no stylesheet of its own should do.
Doxygen has no @readonly, so this is an ALIAS: @styled expands to a remark saying the property is set from a Style and overwritten by the next one applied, and to assign View::style or a stylesheet rule rather than the field. Thirty one properties carry it, across the eight classes that style any. View::frame does not: `left`, `top`, `width` and `height` are bound only when a Style provides them, because layout and any owner positioning its own subviews write it too.
Two fields and a derived third said what one says. View::visibility is the whole story now; hiddenByOwner, the resolveHidden it existed for, and the ViewIsHidden macro that stood in for reading it are all gone, as no other View property has such a macro. View::setVisibility writes View::style as well as the field, the way ScrollView's setter does, so an owner's choice survives the next Style rather than needing a second field to defend it. That makes it imperative, and highest precedence, which is what the element level Style means. Panel hid its accessoryView with the old setHidden, which under that precedence no stylesheet could undo, and a dialog showing one is exactly what a stylesheet is for. It is a class default, and reads as one in Panel .accessoryView.
There was a problem hiding this comment.
🟡 Changes recommended
A few changes still conflict with the PR’s stated “no direct assignment of styled properties” rule and there’s a documentation alias issue that could break generated docs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR refactors ObjectivelyMVC styling so the cascade becomes the sole source of truth for @styled properties, preventing stale values from persisting after selector matches change. It also replaces the legacy hidden boolean with a style-backed visibility model and updates default styling to be driven by CSS declarations.
Changes:
- Makes styled property application “total” by relying on stylesheet/type-selector defaults and ensuring re-application when element Style changes.
- Replaces
View::setHidden/View::hiddenwithView::setVisibility(ViewVisibility)/View::visibility, and updates call sites accordingly. - Expands style shorthands into longhands on write (e.g.,
padding,min-size,max-size) to let the cascade resolve conflicts correctly.
File summaries
| File | Description |
|---|---|
| Sources/ObjectivelyMVC/ViewController.c | Switches autoresizing defaults to style attributes. |
| Sources/ObjectivelyMVC/View.h | Introduces visibility / pointer-events enums, ClampSize, and @styled annotations. |
| Sources/ObjectivelyMVC/View.c | Applies visibility/pointer-events changes and forces style re-apply when invalidated. |
| Sources/ObjectivelyMVC/TextView.h | Marks isEditable as styled. |
| Sources/ObjectivelyMVC/TextView.c | Removes direct assignment of a styled property (clipsSubviews). |
| Sources/ObjectivelyMVC/Text.h | Marks text color as styled. |
| Sources/ObjectivelyMVC/TabViewController.c | Moves autoresizing configuration into the view Style. |
| Sources/ObjectivelyMVC/TableView.c | Updates hidden checks to visibility checks. |
| Sources/ObjectivelyMVC/Style.c | Expands shorthands into longhands when storing attributes. |
| Sources/ObjectivelyMVC/StackView.h | Marks layout-related fields as styled. |
| Sources/ObjectivelyMVC/StackView.c | Uses ClampSize and removes direct styled assignment. |
| Sources/ObjectivelyMVC/Slider.c | Updates hidden checks to visibility checks. |
| Sources/ObjectivelyMVC/Select.c | Updates show/hide logic to visibility API. |
| Sources/ObjectivelyMVC/ScrollView.h | Marks scrollbar visibility as styled. |
| Sources/ObjectivelyMVC/ScrollView.c | Moves scrollbar visibility control into style/invalidation path. |
| Sources/ObjectivelyMVC/ScrollBar.c | Removes direct assignment of a styled property (clipsSubviews). |
| Sources/ObjectivelyMVC/RGBColorPicker.c | Writes background color via Style + invalidation instead of direct field assignment. |
| Sources/ObjectivelyMVC/Renderer.h | Updates gradient angle documentation (south-based). |
| Sources/ObjectivelyMVC/Renderer.c | Adjusts gradient math to match the updated angle convention. |
| Sources/ObjectivelyMVC/Panel.h | Marks panel behavior flags as styled. |
| Sources/ObjectivelyMVC/Panel.c | Updates hidden usage and removes direct styled assignments in init/layout. |
| Sources/ObjectivelyMVC/PageView.c | Migrates page show/hide to visibility API. |
| Sources/ObjectivelyMVC/Input.c | Migrates layout-related configuration to style attributes + invalidation. |
| Sources/ObjectivelyMVC/HueColorPicker.c | Writes background color via Style + invalidation. |
| Sources/ObjectivelyMVC/HSVColorPicker.c | Writes background color via Style + invalidation. |
| Sources/ObjectivelyMVC/Control.h | Marks visual/selection properties as styled. |
| Sources/ObjectivelyMVC/CollectionView.h | Marks layout properties as styled. |
| Sources/ObjectivelyMVC/CollectionItemView.c | Removes direct styled assignments in favor of stylesheet defaults. |
| Sources/ObjectivelyMVC/Checkbox.c | Removes direct styled assignments in favor of stylesheet defaults. |
| Sources/ObjectivelyMVC/Box.c | Updates hidden checks and removes direct styled assignments in init. |
| Examples/HUD.c | Updates example to set styled properties via Style / visibility API. |
| Doxyfile | Adds a styled alias for generated docs. |
| configure.ac | Bumps package version to 2.5.0. |
| Assets/stylesheet.css.h | Updates embedded stylesheet bytes to match new defaults/rules. |
| Assets/stylesheet.css | Adds universal/type defaults to support full style reset and new properties. |
Review details
- Files reviewed: 35/35 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The View suite still spoke the pre-cascade API, so it did not compile and `make check` never ran on this branch: it called the removed setHidden, assigned pointerEvents a bool where the enum's zero is now `auto`, and sized a Contain view with the `width` and `height` attributes that min-width and min-height replaced. ScrollView::initWithFrame assigned scrollBarVisibility, which is @styled; the `ScrollView` rule in the stylesheet already supplies `scrollbar: auto`. Spell the Doxygen alias @remarks, as the rest of the headers do.
…class The accessory view starts hidden until a Panel is given something to put in it, which was an assignment in Panel::initWithFrame until the cascade took the styled properties over. Moving it to `Panel .accessoryView` made it a property of the class name, and consumers wear that name on views of their own to pick up its layout -- so all of those went hidden too. Address the Panel's own by where the Panel puts it instead, leaving the class name to carry the layout it is worn for.
ViewController::loadView wrote `autoresizing-mask: fill` into the View's own style, which the cascade merges last, so no stylesheet could override it: a controller whose view is a page measured by its content, or a panel of its own, was stuck filling its parent. A `fill` child measures as nothing to a `contain` parent, so a TabView of such pages collapsed, and the Panel around it closed up on its tab bar. The root View takes the class name `viewController` and the stylesheet gives that `fill`, which a consumer's own rule can outrank.
The box holds a 12 pixel check in 4 pixels of padding, so it contains at 20 -- but its floor was 18, which is what it fell to whenever the check was hidden. Selecting a checkbox grew it by two pixels and shifted whatever was laid out beside it.
A Style that stopped matching went on applying.
View::applyStylewrote the properties thecascade provided and left the rest holding whatever the last Style had set, so a closed
Selectkept the padding its open flyout had, and a class removed left its values behindforever.
Resolution is total now: every application starts from the same place, and the cascade decides
the whole result. The defaults that make that possible are declarations rather than code — the
universal selector carries
View's, a type selector carries each subclass's — andView::initWithFrameno longer sets a single styled property.What changes for callers
Nothing may assign a styled property in C. The cascade overwrites it on the next
application. Assign the View's own
Style, which is merged last and so always wins, or writea rule. Thirty one properties are marked
@styledto say so, via a DoxygenALIASESentry.View::setHiddenis nowView::setVisibility(View *, ViewVisibility).View::hiddenandView::hiddenByOwnerare gone;View::visibilityis the whole story. The setter writes theelement level Style, so an owner's choice outlives the next application, and outranks a
stylesheet rule — which is what an imperative call should mean.
frameis no longer a style attribute, andView::styledSizeandView::styledOriginare gone with it.
left,top,widthandheightwrite the frame directly and are boundonly when a Style provides them, because layout and any owner positioning its own subviews
write it too. A container that needs a size its content cannot give it says
min-widthnow.Two defaults became zero so the stylesheet could hold them. A
maxSizeof zero meansunbounded, per axis, so
max-width: 460alone leaves the height free. The gradient angle ismeasured from south, so
0fills downwards;90and270are unchanged.Shorthands are stored as longhands.
paddingandpadding-leftname the same bytesthrough different Inlets, and Inlet declaration order, not the cascade, decided which won: a
class rule setting
min-heightlost to a type rule settingmin-size. Splitting them on theway in lets the cascade resolve each on its own.
Verified
Ten harnesses covering the reset, the cascade, visibility precedence, owner-positioned views,
pre-Theme construction, and the styled properties reaching every widget. Both examples run
clean. Every commit builds standalone.
The layout regressions found along the way, each now covered by a test: an
Input's axisfalling back to vertical, a
Select's flyout andCollectionView's items snapping to theorigin, a
Boxlabel losing its offset, aSelectselecting nothing until clicked.🤖 Generated with Claude Code