Repository navigation
feat: Deliver eXIP 7.3.0.31 Icon Colorization EXO-90342 - #579
Conversation
…90337 (#576) **Why** eXIP 7.3.0.31 US04 (EXO-90337, note 51586 §4 *Access*, *Renderers*, §2 *Security*): the Edit Page drawer and the application editor offer the Icon row of the Text section, and saving applies the colour to that page's icons or to that app's icons only. The site banner, sidebar and application drawers render the same shared input, so they get the row too. **How** - The five drawers pass `custom-icon` to the shared text input; the row stays empty when the section is switched on and stores a value only when the user picks one (D6). - `LayoutModel` and `EntityMapper` carry `iconColor` like the text colour; a container whose only custom value is the icon colour still gets a `css-style`. - `ApplicationUtils.getStyle` emits `--appIconColor` where it emits `--appTextColor`, so a page value is inherited by its applications and an application value wins; `LayoutUtils.applyContainerStyle` copies the field. - `LayoutStyleValidator` (new): the page save's style validation, moved as is and shared with `SiteLayoutService.updateSiteLayout`, which validated nothing before; the icon colour is hex-only (D7), and a sticky section's `top@middle` scroll colours are checked half by half. `SiteLayoutRest` maps the refusal to a 400 as the page endpoint does. - Tests: the model round trip, the hex-only refusal on page and site, the sticky colours accepted, the 400 on the site endpoint. Depends on Meeds-io/portal's `ModelStyle.iconColor` under the same task id. Knowledge: owed by the eXIP's knowledge update (EXO-90341), gated at the `feature/mips` integration PR. This change is classified N1 at bd41ff7: the validator between editor-supplied style values and the inline CSS every viewer receives is rewritten and widened to the site save path. Its approver must be an Archi/Dev who knows it is N1, not an approval on AI review alone, and must not be the author. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 4a857b6)
…our EXO-90340 (#577) **Why** eXIP 7.3.0.31 US07 (EXO-90340): every icon honours the configured icon colour. The site-navigation icons that still carried a neutral hard-coded colour are rebased on platform-ui's `@iconColor` chain, `var(--appIconColor, var(--allPagesBaseIconColor, <built-in>))`, so they follow the container keys of US02 and US03 and keep the platform grey when none is set. **How** - `NodeItem.vue`: the visibility and access icons lose `color="grey"` and `dark` (which would have painted them white). Left as they are: `disabled--text` on system items, success, error, white and primary icons. Verified: Less compiles and the war builds; deployed on a local server. Knowledge: owed at the `feature/mips` integration PR — the US07 audit's two lessons (a text helper class painting an icon outside the chain, an SVG image used as an icon) go to the eng-standards PR the delivery owes. This change is classified N3 at c66a3cb: no REST/DAO/schema/ACL surface touched (`classification.md` §3), stylesheet and template changes only; the delivery is N1 by max-severity at the `feature/mips` integration PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 2278e4b)
…herits EXO-90337 (#578) **Symptom** With a Page & Apps icon colour set in Branding, the Edit Page drawer's Icon row shows its dimmed `Inherited` swatch in the built-in grey, not the configured colour; the application editor shows the grey too once the page stores an icon colour (EXO-90337, PO feedback on US04). **Cause** The shared text input (Meeds-io/social, `styling-inputs/TextInput.vue`) passed the built-in `#707070` as the Icon row's placeholder at every level, by design of note 51586 §4 (D6). The PO ruled on EXO-90337 that the page-level row shows the platform's customised colour and the application-level row shows the page's; nothing is stored until the user picks (D6 kept). The editors never resolved that colour. **Fix** - `ApplicationUtils.getThemeColor(name)`: the value of a branding theme variable as computed at the document root, null when the branding stylesheet emits `initial`. - `EditPageDrawer.vue`: the platform's `--allPagesAppIconColor`. - `common-layout/EditApplicationDrawer.vue`: the page root container's stored icon colour, else the platform's. - The site layout editor drawers are unchanged and keep the built-in grey. Known limit, left to the PO: a colour the site stores on its own page area (the site layout editor's page properties, `middleCenterContainer`) is inherited by every page of the site at view time but is not read by the page editor, which renders the page alone, outside the site layout that carries that colour, and reads the theme at the document root; the page and application placeholders then show the platform's colour, and covering it needs a fetch of the site layout at open. Depends on the `iconPlaceholder` prop of the shared text input in Meeds-io/social, same task id. Verified with eslint on the changed files and a headless Chrome check of `getThemeColor`: an `initial` custom property computes to the empty string, a set one and `transparent` come back as written. No committed JS test (frontend-vue.md). Knowledge: owed by the eXIP's knowledge update (EXO-90341), gated at the `feature/mips` integration PR. This change is classified N3 at a7b90cd: editor-side placeholder only, no REST/DAO/schema/ACL surface touched (`classification.md` §3); the delivery is N1 by max-severity at the `feature/mips` integration PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 116aacc)
boubaker
left a comment
boubaker
left a comment
There was a problem hiding this comment.
AI review — Round #1
eXIP 7.3.0.31 Icon Colorization — the 14 integration PRs to feature/mips (EXO-90342) are reviewed as one delivery against the Tech Spec (note 51586) and board 8381; findings are reported on the PR they belong to.
Integration checked for this PR: every feature/devx commit of the eXip in this repo is carried (patch-id equal), and every touched file is byte-identical to origin/feature/devx; eXIP 7.3.0.30, the prerequisite, is on develop. The Knowledge: line's eng-standards #222, #233 and #321 are open drafts whose files match the body, as pr-conventions.md §3 wants until develop.
Delivery-level items (two missing integration PRs in agenda and onlyoffice, no commons-exo change, four inventory repos to confirm): see the Round #1 review on platform-ui#1020.
Spec resync (D8, the placeholder in the page, app and site editors): see the Round #1 review on social#6199.
🟢 Nits
ApplicationUtils.js:135-137writes--appIconColorfor any container carryingiconColor. R6 ("sections and cells never carry an icon colour") holds only because no editor offers the row there: a rawPUT /layout/rest/pages/layoutcan put one on a section, and it then inherits into the apps below. The value is hex-validated, so this is harmless; noted only so the spec's statement is read as an editor rule.// eXIP 7.3.0.31:prefixes inPageLayoutServiceTest,SiteLayoutServiceTestandLayoutModelTest: same as portal, keep the sentence and drop the tag.
Test evidence at this head, @MayTekayaa: please post the result line at 31911763ff: mvn -B -ntp -pl layout-service -Dtest='PageLayoutServiceTest,SiteLayoutServiceTest,SiteLayoutRestTest,LayoutModelTest' -Dsurefire.failIfNoSpecifiedTests=false. The tests were read at head and assert what the body claims.
Verified conform
LayoutStyleValidator is called from createPage, updatePageLayout and SiteLayoutService.updateSiteLayout; on the site path it runs after canEditSite, so the order existence → ACL → validation holds; it recurses into children; iconColor has its own hex-only check with matches(), no keyword, and is kept out of the generic matcher (J12, J16); the swap from the jarjar re2j Pattern to java.util.regex keeps full-match semantics; the site's sticky <top>@<middle> background pair is split and each half validated, matching the renderer, instead of breaking every sticky-site save; IllegalArgumentException → 400 on the new site REST path; the other save paths take no client style; --appIconColor is bound only through the Vue style object; EntityMapper keeps a style whose only value is the icon colour (pinned); tests cover accept and reject on both paths, recursion into a child section and the REST 400.
What this PR does well
The validator is one shared class on both save paths, as D7 asked, and the site's @ pair is handled rather than silently refused.
Classification: N1 — confirmed over the full eXip diff: administrator- and editor-entered colour values rendered for every viewer (the branding stylesheet, the inline --appIconColor), and LayoutStyleValidator now on the site layout save path. This PR must be approved by an Architect/Senior Developer who knows it is N1, never auto-merged on AI review alone; author ≠ approver.
🤖 Generated with Claude Code
…0342 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Round #1 — author's replies
Test evidence at 1fcd87c ( |
boubaker
left a comment
boubaker
left a comment
There was a problem hiding this comment.
AI review — Round #2 (follow-up)
| Round #1 finding | Status |
|---|---|
🟢 // eXIP 7.3.0.31: prefixes in tests |
✅ 1fcd87c, verified (comment-only) |
🟢 R6 enforced by the editors only (ApplicationUtils.js:135-137) |
➖ Accepted: R6 read as an editor rule, and the value stays hex-validated on the save path |
| Test evidence at the head | ✅ Read: PageLayoutServiceTest, SiteLayoutServiceTest, SiteLayoutRestTest, LayoutModelTest 65/0/0 at 1fcd87c |
🟢 Nit (new) — the body still says the commits are "diff-identical to the validated devx state on every touched file". That is no longer true for the file the declared commit changes. Say it of the cherry-picks only, e.g. "the cherry-picks are diff-identical to devx; the declared commits below are review fixes made on this branch".
Delivery-level status (the eXoSkin scope question with the PO, the Architects Lead's ruling on chat-application, the PO notification for the Mips ACC tests): see Round #2 on platform-ui#1020.
Classification: N1 (confirmed over the full eXip diff: administrator- and editor-entered colour values rendered for every viewer, LayoutStyleValidator on the site layout save path). This PR must be approved by an Architect/Senior Developer who knows it is N1, never auto-merged on AI review alone; author ≠ approver.
🤖 Generated with Claude Code
boubaker
left a comment
boubaker
left a comment
There was a problem hiding this comment.
AI review — Round #3 (final)
| Previous finding | Status |
|---|---|
| 🟢 "diff-identical" sentence | ✅ Body updated |
All findings from previous rounds are resolved; nothing is outstanding on this PR from the AI review side. The body now states the release order (platform-ui → commons-exo → portal → social → layout → digital-workplace, then the audit PRs) and says "diff-identical to devx" of the cherry-picks only. The delivery-level items are tracked on platform-ui#1020.
Classification: N1 (confirmed over the full eXip diff: administrator- and editor-entered colour values rendered for every viewer, LayoutStyleValidator on the site layout save path). This PR must be approved by an Architect/Senior Developer who knows it is N1, never auto-merged on AI review alone; author ≠ approver.
🤖 Generated with Claude Code
2 participants
eXIP 7.3.0.31 — Icon Colorization
Integration PR of the eXip onto
feature/mips. Tech Spec: note 51586 — board: project 8381 (every story Tested & Validated on the devx acceptance server; US08's D8 and the D6 placeholder ruling await the spec resync).Classification: N1 at 1fcd87c — the
/classifyrun over the full eXip diff of the delivery branches againstfeature/mips, confirming the Tech Spec's proposal: administrator- and editor-entered colour values rendered for every viewer (the branding stylesheet, the inline--appIconColor), with layout'sLayoutStyleValidatorrewritten on that trust boundary and extended to the site layout save; the portal attribute and theme keys N2 on their own, the rest N3 — max-severity over the full diff. Perai-review-and-merge.md§5: the approver must be an Architect/Senior Developer who knows this is N1, not an approval on AI review alone; author ≠ approver.Release order: platform-ui → commons-exo → portal → social → layout → digital-workplace, then the audit PRs (agenda, analytics, app-center, content, documents, gamification, notes, onlyoffice, processes, task, wallet) in any order. One PR per repo, same eXip; the cherry-picks are
cherry-pick -xof thefeature/devxcommits ontofeature/mips, diff-identical to the validated devx state on every touched file, no POM touched; the declared commits listed below, where present, are review fixes made on this branch.Knowledge: Meeds-io/eng-standards#222 and #233 (the ledger pitfalls of the eXip, drafts until
develop), and Meeds-io/eng-standards#321 (the domain refresh of portal, platform-ui, social and layout with four delivery checks, draft untildevelop).This repo: the Icon row in the page, application and site section editors (placeholder = the colour the level inherits, read with
getThemeColorat the document root or from the parent container),--appIconColorwritten bygetStyle,LayoutStyleValidatorvalidating every style value on the page and the site layout save paths (hex-only icon colour, the<top>@<middle>background pair, 400 fromSiteLayoutRest), and the site navigation visibility icons on the cascade.Commits (cherry-picked from
feature/devx)Declared commits (fixes on this branch, round 1)
🤖 Generated with Claude Code