Skip to content

Set embedded app theme to match the site theme #1387

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.

Sign up for GitHub

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

Jump to bottom
Merged
sfc-gh-dmatthews merged 7 commits into from
Dec 17, 2025
Merged

Set embedded app theme to match the site theme #1387

sfc-gh-dmatthews merged 7 commits into from
Dec 17, 2025
+181 −46

Conversation

@sfc-gh-dmatthews
Copy link
Contributor

@sfc-gh-dmatthews sfc-gh-dmatthews commented Dec 8, 2025

📚 Context

This PR passed the currently selected theme to all Community Cloud embedded apps as a query parameter to make them match the site theme.

Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@sfc-gh-dmatthews sfc-gh-dmatthews requested review from a team and removed request for sfc-gh-kmcgrady December 8, 2025 15:17
sfc-gh-tteixeira
sfc-gh-tteixeira approved these changes Dec 8, 2025
View reviewed changes
Copy link
Contributor

@sfc-gh-tteixeira sfc-gh-tteixeira left a comment

Choose a reason for hiding this comment

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

Approved after comments addressed

@sfc-gh-dmatthews
Copy link
Contributor Author

sfc-gh-dmatthews commented Dec 9, 2025 •
edited
Loading

I think I followed your suggestions. I couldn't get rid of the function entirely, because server-side rendering broke. So, re-requested a review to make sure it satisfies you requests.

@sfc-gh-dmatthews
Copy link
Contributor Author

sfc-gh-dmatthews commented Dec 11, 2025

Actually, the breaks a portion. Apps from docstrings only load in light mode. I'll update it again.

@sfc-gh-dmatthews sfc-gh-dmatthews force-pushed the fix/set-apps-from-site-theme branch from 95fdbe4 to a2a8a40 Compare December 14, 2025 01:53
@sfc-gh-dmatthews
Copy link
Contributor Author

sfc-gh-dmatthews commented Dec 14, 2025 •
edited
Loading

I changed it to use a theme context to so that API pages would load with the correct theme. The first fix had the following problem:

  1. Load in light theme
  2. Switch to dark theme
  3. Navigate to API page
  4. Apps were light, but could toggle to light and back to dark to make them dark

The new fix will load the right theme for the API apps, even if the theme was changed sometime after the first load.

sfc-gh-tteixeira
sfc-gh-tteixeira approved these changes Dec 17, 2025
View reviewed changes
Copy link
Contributor

@sfc-gh-tteixeira sfc-gh-tteixeira left a comment

Choose a reason for hiding this comment

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

I just pushed a change to avoid duplicating code.

Please double-check it and LMK if I broke anything, though!

sfc-gh-tteixeira
sfc-gh-tteixeira reviewed Dec 17, 2025
View reviewed changes
@sfc-gh-dmatthews
Copy link
Contributor Author

sfc-gh-dmatthews commented Dec 17, 2025 •
edited
Loading

LGTM! Okay to merge, @sfc-gh-tteixeira? Edit: I see you re-approved, so I think so!

@sfc-gh-dmatthews sfc-gh-dmatthews merged commit 290f9cc into main Dec 17, 2025
4 checks passed
@sfc-gh-dmatthews sfc-gh-dmatthews deleted the fix/set-apps-from-site-theme branch December 17, 2025 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfc-gh-tteixeira sfc-gh-tteixeira sfc-gh-tteixeira approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sfc-gh-dmatthews @sfc-gh-tteixeira