-
Notifications
You must be signed in to change notification settings - Fork 2
UI branding #193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
UI branding #193
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
cb99386
feat: add runtime UI branding customization
491abe0
feat: add support for favicon icon
39b5e62
feat: make dark mode brandable via tokenized palette
4aaa303
docs: add operator branding guide
f90d6aa
fix: use plain strings over pointers in branding
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,85 @@ | ||
| // Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. | ||
| // SPDX-License-Identifier: BSD-3-Clause-Clear | ||
|
|
||
| package web | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "log/slog" | ||
| "regexp" | ||
| ) | ||
|
|
||
| // cssColor allows hex, rgb()/rgba()/hsl() functional notation, and CSS named | ||
| // colors — i.e. the characters those need. Anything else (e.g. ";", "{") is | ||
| // rejected so a bad branding.json can't inject or break the stylesheet. | ||
| var cssColor = regexp.MustCompile(`^[#a-zA-Z0-9(),.%\s/-]+$`) | ||
|
|
||
| // Branding holds the resolved branding values (defaults applied) used by both | ||
| // HTML templates and the templated CSS. | ||
| type Branding struct { | ||
| Title string // app name in topbar + <title> suffix | ||
| Logo string // filename under <data-dir>/branding/, "" = text brand only | ||
| Primary string // --brand-primary | ||
| Accent string // --brand-accent | ||
| Surface string // --surface-1 (page background) | ||
| SurfaceAlt string // --surface-2 (content background) | ||
| Text string // --text-1 | ||
| } | ||
|
|
||
| // brandingFile is the on-disk JSON shape. Pointers distinguish "absent" (keep | ||
| // default) from "explicitly set". | ||
| type brandingFile struct { | ||
| Title *string `json:"title"` | ||
| Logo *string `json:"logo"` | ||
| Colors struct { | ||
| Primary *string `json:"primary"` | ||
| Accent *string `json:"accent"` | ||
| Surface *string `json:"surface"` | ||
| SurfaceAlt *string `json:"surface-alt"` | ||
| Text *string `json:"text"` | ||
| } `json:"colors"` | ||
| } | ||
|
|
||
| func defaultBranding() Branding { | ||
| return Branding{ | ||
| Title: "Foundries Update Server", | ||
| Logo: "", | ||
| Primary: "rgb(2, 11, 64)", | ||
| Accent: "#a3b4ff", | ||
| Surface: "#f2f2f2", | ||
| SurfaceAlt: "#ffffff", | ||
| Text: "#000000", | ||
| } | ||
| } | ||
|
|
||
| // LoadBranding applies any overrides in the given branding.json bytes onto the | ||
| // defaults. nil/empty input or a parse error yields the defaults unchanged. | ||
| func LoadBranding(data []byte) Branding { | ||
| b := defaultBranding() | ||
| if len(data) == 0 { | ||
| return b | ||
| } | ||
| var f brandingFile | ||
| if err := json.Unmarshal(data, &f); err != nil { | ||
| slog.Warn("ignoring invalid branding.json, using defaults", "error", err) | ||
| return b | ||
| } | ||
| set := func(dst *string, src *string) { | ||
| if src != nil { | ||
| *dst = *src | ||
| } | ||
| } | ||
| setColor := func(dst *string, src *string) { | ||
| if src != nil && cssColor.MatchString(*src) { | ||
| *dst = *src | ||
| } | ||
| } | ||
| set(&b.Title, f.Title) | ||
| set(&b.Logo, f.Logo) | ||
| setColor(&b.Primary, f.Colors.Primary) | ||
| setColor(&b.Accent, f.Colors.Accent) | ||
| setColor(&b.Surface, f.Colors.Surface) | ||
| setColor(&b.SurfaceAlt, f.Colors.SurfaceAlt) | ||
| setColor(&b.Text, f.Colors.Text) | ||
| return b | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| // Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. | ||
| // SPDX-License-Identifier: BSD-3-Clause-Clear | ||
|
|
||
| package web | ||
|
|
||
| import "testing" | ||
|
|
||
| func TestBrandingDefaultsWhenEmpty(t *testing.T) { | ||
| if got := LoadBranding(nil); got != defaultBranding() { | ||
| t.Errorf("LoadBranding(nil) = %+v, want defaults %+v", got, defaultBranding()) | ||
| } | ||
| } | ||
|
|
||
| func TestBrandingOverrides(t *testing.T) { | ||
| input := []byte(`{"title":"Acme","logo":"logo.svg","colors":{"primary":"#cc0000"}}`) | ||
| b := LoadBranding(input) | ||
| if b.Title != "Acme" { | ||
| t.Errorf("Title = %q, want Acme", b.Title) | ||
| } | ||
| if b.Primary != "#cc0000" { | ||
| t.Errorf("Primary = %q, want #cc0000", b.Primary) | ||
| } | ||
| if b.Logo != "logo.svg" { | ||
| t.Errorf("Logo = %q, want logo.svg", b.Logo) | ||
| } | ||
| if b.Accent != "#a3b4ff" { | ||
| t.Errorf("Accent = %q, want default", b.Accent) | ||
| } | ||
| } | ||
|
|
||
| func TestBrandingInvalidJSONFallsBackToDefaults(t *testing.T) { | ||
| if got := LoadBranding([]byte(`{not valid`)); got != defaultBranding() { | ||
| t.Errorf("LoadBranding(invalid) = %+v, want defaults %+v", got, defaultBranding()) | ||
| } | ||
| } | ||
|
|
||
| func TestBrandingRejectsMaliciousColor(t *testing.T) { | ||
| input := []byte(`{"colors":{"primary":"red; } body { display:none } :root {"}}`) | ||
| b := LoadBranding(input) | ||
| if b.Primary != defaultBranding().Primary { | ||
| t.Errorf("Primary = %q, want default (malicious value rejected)", b.Primary) | ||
| } | ||
| } | ||
|
|
||
| func TestBrandingAcceptsValidColors(t *testing.T) { | ||
| input := []byte(`{"colors":{"primary":"rgb(10, 20, 30)","accent":"hsl(200 50% 50%)","text":"#abc"}}`) | ||
| b := LoadBranding(input) | ||
| if b.Primary != "rgb(10, 20, 30)" || b.Accent != "hsl(200 50% 50%)" || b.Text != "#abc" { | ||
| t.Errorf("valid colors rejected: %+v", b) | ||
| } | ||
| } |
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I get the value of having two different structs? I think I'd stick with just the brandingFile idea but get rid of the pointers do length == 0 checks instead checks for nil.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pointers are overkill, yeah. I'll switch to plain strings and drop the nil checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the two structs, you mean
brandingFileandBranding?In that case I think it's still worth keeping them both, as they serve different purposes.
brandingFilemirrors the raw JSON of thebranding.jsonfile used to customize the UI colors.Brandingis the flat version, template-friendly with defaults applied: this is guaranteed to be populated and valid viaLoadBranding.Staying only with brandingFile would lead to move the "default value resolution/validation" logic into the templates, and we might have to duplicate it in multiple places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking more of having something like an "applyDefaults" method added to the brandingFile struct. Then pass that to the templates and everything would be in place for it?
If that approach looks bad - stick with this. I'm fine either way. Merge at your leisure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only reasons I still prefer the two structs approach is that the only way to get the branding is via calling that
LoadBrandingfunc. WithapplyDefaultsand a single struct there would be less code, but there's nothing stopping a caller to forget to call it, handing down to a template a struct with non-default values.I'll keep it as is for now, but we can rework it later if it feels too much.