Skip to content

Improve the way we create a base page context#202

Merged
doanac merged 1 commit into
mainfrom
branding-fix
Jul 13, 2026
Merged

Improve the way we create a base page context#202
doanac merged 1 commit into
mainfrom
branding-fix

Conversation

@doanac

@doanac doanac commented Jul 10, 2026

Copy link
Copy Markdown
Member

We've had this split where the auth package and UI package have their own logic for creating this base page context.

Every time we've added a new field, this time its branding stuff, we've broken some page's rendering logic. This change introduces logic to consolidate things and fights circular dependencies.

There are even better ways this could be done. However, this is the least invasive way and probably serves the size of our code well enough to survive with for the foreseeable future.

@doanac doanac requested review from MiloCasagrande and vkhoroz July 10, 2026 18:28

@vkhoroz vkhoroz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread server/ui/web/handlers.go
CsrfToken: csrfToken,
Version: version.Version,
}
return h.pages.Base(c, title, selected)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify on the commit message: we were usually breaking the auth pages, not literally "every page" when introducing new base context fields, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's correct

We've had this split where the auth package and UI package have their
own logic for creating this base page context.

Every time we've added a new field, this time its branding stuff, we've
broken some page's rendering logic. This change introduces logic to
consolidate things and fights circular dependencies.

Signed-off-by: Andy Doan <doanac@qti.qualcomm.com>
Co-authored-by: GitHub Copilot:claude-opus-4.8 <noreply@github.com>
@doanac

doanac commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

rebased on latest main

@doanac doanac merged commit 3248f34 into main Jul 13, 2026
14 checks passed
@doanac doanac deleted the branding-fix branch July 13, 2026 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants