Repository navigation
Commit 0093b2c
fix(console): a settings value that misses its declared format blocks Save (objectui#13287) (#13298)
Fixes #13287
Clause-②: no
## What changed
On Save, the Settings page now judges every visible, unlocked `url`,
`email`, `phone` and `color` row whose effective value (draft over
stored) is non-empty. A miss blocks the save on the same path as the
`required` check from objectui#13239: no request goes out, each row is
marked inline through the `fieldErrors` slot (`aria-invalid` plus
`aria-describedby` to the message), focus moves to the first blocked row
in manifest order, and the existing blocked-save toast names the rows.
An empty value is left to `required`.
- `apps/console/src/pages/settings/SettingsView.tsx`:
`emptyRequiredKeys` becomes `preSaveBlocks`. It runs one pass with the
same visibility, lock and non-input skips, and returns `required` or
`format` per key. `DECLARED_VALUE_FORMATS` with `missesDeclaredFormat`
does the judging, on the trimmed value.
- Messages, one per format: `email` reuses `validation.email` and `url`
reuses `validation.url`. Two new keys,
`console.settingsView.invalidPhone` and
`console.settingsView.invalidColor`, name what a phone number and a hex
colour take. Both are added to all ten locale packs.
- A server `SETTINGS_VALIDATION` refusal renders as before. That branch
is untouched.
## Grammars: the settings service's own, copied from objectstack main
They come from `DECLARED_VALUE_FORMATS` in `service-settings`
(objectstack PR #25022, merged), so the page never blocks a value the
server accepts and never passes one it refuses:
- `email`: `local@domain` with a dotted domain.
- `url`: `scheme://...`, root-relative `/path`, `data:` or `blob:`.
**This is wider than the dispatch's gloss "scheme http/https".** The
server accepts any `scheme://`. The `branding.logo_url` reader takes
absolute or relative values, and platform storage hands back
root-relative file URLs. An http(s)-only console would block values the
server and the reader both serve.
- `phone`: digits, spaces and `+ ( ) - .`, at least 5 characters. The
server has this grammar, so `phone` is checked rather than left out.
- `color`: `#rgb`, `#rgba`, `#rrggbb` or `#rrggbbaa`.
## Acceptance notes
- **Untouched stored values.** The page judges the effective value, so a
stored malformed value on a visible row blocks a save that edits a
different row. That row is shown marked so the admin can fix it. This is
what the dispatch ruling asks ("effective value"), and the `required`
check works the same way. The server, by contrast, judges only the keys
a save touches. It is pinned by the test "a stored malformed value on a
visible row blocks a save that edits another row".
- **Colour rows.** On a `color` row, `aria-invalid` and
`aria-describedby` land on the wrapper `div`, not on the two inputs.
This is how `SettingsField`'s `wrapper` already handles a server refusal
on that row type: `cloneElement` applies to the outer child. The message
itself still renders in the row, and focus goes to the picker. This was
found by reading the code and has not been measured in a browser. It is
not changed here.
## Tests (at `a1dd56fed7`)
- New:
`apps/console/src/pages/settings/__tests__/SettingsView.formatCheck-13287.test.tsx`,
15 tests.
- (a) Logo URL `not a url` blocks: no request, row marked, focus on it.
`https://...`, `/logo.png` and a padded `/logo.png` all save.
- (b) `no-reply@localhost` blocks; a valid email saves.
- (c) `red` and `#12` block; `#1a2b3c` saves.
- Phone: a value with letters blocks, a valid one saves. Four misses get
four distinct messages, with focus on the first.
- (d) control: an empty optional url saves.
- (e) control: a url hidden by `visible` and an env-locked malformed url
do not block.
- Editing a marked row clears its message.
- `pnpm exec vitest run --maxWorkers=1
apps/console/src/pages/settings/`: `Test Files 28 passed (28)`, `Tests
233 passed (233)`. This covers the family that reads `SettingsView.tsx`,
including the required-check suite.
- The 56 `packages/i18n/src/__tests__` files that read the locale packs:
`Test Files 56 passed (56)`, `Tests 1003 passed (1003)`. The full
`apps/console` and `packages/i18n` directories are left to CI.
- Ablation via `ablation-replace.mjs`, run at the commit above. The
mutation `} else if (missesDeclaredFormat(` became `} else if (false &&
missesDeclaredFormat(`; anchor count went 1 to 0 and the blob went
`4f3bc9f9bb64` to `c0bd531ba548`. Result: 8 failed, 7 passed. Every (a),
(b) and (c) block test went red; the save cases and the (d) and (e)
controls stayed green. Restore: blob `4f3bc9f9bb64` equals the HEAD blob
and `git diff HEAD` is empty.
- `type-check` for `@object-ui/i18n` and `@object-ui/console`, after the
`@object-ui/console^...` closure build: both `Done`, exit 0. Console
`tsc --listFiles` includes the new test.
- `eslint` over the 12 changed files (JSON count 12): 0 errors and 3
warnings, all `no-explicit-any` on pre-existing `catch (err: any)`
lines. With `--no-inline-config` there is one more, the pre-existing
suppressed `react-hooks/static-components`. The base file gives the same
four findings. Type-aware linting is not configured in
`eslint.config.js`, so this diff cannot change the verdict on any
untouched file.
- Gates, all exit 0: `check:phantom-deps`, `check:self-import`,
`check:unreferenced-sources`, `check:doc-example-readers`,
`check:handler-key-reads`, `check:side-effects-array`,
`check:element-data-source-declaration`, `check:spec-symbols`,
`check:action-forward-parity`, `check:designer-field-key-parity`,
`check:icon-record-names`, `check:i18n-keys`, `check:i18n-drift`,
`check:i18n-designer-parity`, `check:i18n-dead-keys`,
`check:control-bytes`, `scripts/check-hand-rolled-comment-mask.mjs` and
`scripts/check-changeset-fixed.mjs`.
Changeset: `.changeset/13287-settings-format-check.md`
(`@object-ui/console` patch, `@object-ui/i18n` patch).
---
_Generated by [Claude
Code](https://claude.ai/code/session_01XVp19E1MYUTFM3dnkwz8gB)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent 7c405b7 commit 0093b2c
13 files changed
Lines changed: 365 additions & 17 deletions
File tree
- .changeset
- apps/console/src/pages/settings
- __tests__
- packages/i18n/src/locales
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
148 | 148 | | |
149 | 149 | | |
150 | 150 | | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
155 | 197 | | |
156 | 198 | | |
157 | 199 | | |
| |||
160 | 202 | | |
161 | 203 | | |
162 | 204 | | |
163 | | - | |
| 205 | + | |
164 | 206 | | |
165 | | - | |
| 207 | + | |
166 | 208 | | |
167 | 209 | | |
168 | 210 | | |
169 | 211 | | |
170 | | - | |
171 | | - | |
| 212 | + | |
| 213 | + | |
172 | 214 | | |
173 | 215 | | |
174 | | - | |
| 216 | + | |
175 | 217 | | |
176 | 218 | | |
177 | 219 | | |
178 | 220 | | |
179 | | - | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
180 | 226 | | |
181 | | - | |
| 227 | + | |
182 | 228 | | |
183 | 229 | | |
184 | 230 | | |
| |||
341 | 387 | | |
342 | 388 | | |
343 | 389 | | |
344 | | - | |
345 | | - | |
346 | | - | |
347 | | - | |
348 | | - | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
349 | 403 | | |
350 | | - | |
| 404 | + | |
351 | 405 | | |
352 | 406 | | |
353 | 407 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
0 commit comments