-
Notifications
You must be signed in to change notification settings - Fork 148
fix: enable real-time live preview for color settings (#1175) #1380
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
base: enext
Are you sure you want to change the base?
fix: enable real-time live preview for color settings (#1175) #1380
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnables real-time theme color live preview by emitting color changes continuously from the ColorPicker component and immediately re-applying theme configuration via a deep watcher on theme colors. Sequence diagram for real-time theme color live previewsequenceDiagram
actor AdminUser
participant ColorPicker
participant AdminThemeView
participant VueWatcher
participant ThemeService
participant BrowserCSS
AdminUser ->> ColorPicker: Drag color input (native input event)
ColorPicker -->> AdminThemeView: emit update:modelValue(newColor) on input
AdminThemeView ->> AdminThemeView: update config.theme.colors via v-model
AdminThemeView -->> VueWatcher: config.theme.colors changed
VueWatcher ->> ThemeService: applyThemeConfig(themePayload)
ThemeService ->> BrowserCSS: Update CSS variables for theme
BrowserCSS -->> AdminUser: Live preview updates immediately
loop Continuous drag
AdminUser ->> ColorPicker: Move picker handle
ColorPicker -->> AdminThemeView: emit update:modelValue(newColor)
AdminThemeView -->> VueWatcher: config.theme.colors changed
VueWatcher ->> ThemeService: applyThemeConfig(themePayload)
ThemeService ->> BrowserCSS: Update CSS variables
end
Flow diagram for color changes to theme CSS variablesgraph TD
A["AdminUser drags color in ColorPicker"] --> B["ColorPicker emits update:modelValue on input"]
B --> C["AdminThemeView updates config.theme.colors"]
C --> D["Vue deep watcher on config.theme.colors triggers"]
D --> E["Build themePayload with merged defaults"]
E --> F["applyThemeConfig(themePayload)"]
F --> G["CSS variables updated in browser"]
G --> H["Live theme color preview visible to AdminUser"]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- The watcher is declared only on
config.theme.colorsbut the handler also readslogo,identicons, andstreamOfflineImage, so consider expanding the watch source (or using a single deep watch onconfig.theme) to keep all theme aspects in sync for live preview. - The watcher’s handler reconstructs the
themePayloadinline; if similar logic exists elsewhere (e.g., save/apply routines), consider extracting a shared helper to avoid duplication and keep future changes to the theme structure in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The watcher is declared only on `config.theme.colors` but the handler also reads `logo`, `identicons`, and `streamOfflineImage`, so consider expanding the watch source (or using a single deep watch on `config.theme`) to keep all theme aspects in sync for live preview.
- The watcher’s handler reconstructs the `themePayload` inline; if similar logic exists elsewhere (e.g., save/apply routines), consider extracting a shared helper to avoid duplication and keep future changes to the theme structure in one place.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
58bd5fc to
6ddb416
Compare
|
Thank you for the excellent suggestions, @sourcery-ai! I've refactored the code to address both points:
|
mariobehling
left a comment
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.
Please share screenshots for PRs that involve UI features or changes. Thank you
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.
Pull request overview
This PR enables real-time live preview for color settings in the theme configuration admin UI. Previously, color changes only applied after clicking Save; now they update instantly while editing.
Key Changes:
- Modified ColorPicker to emit updates continuously during color selection via
@inputinstead of@change - Added a deep watcher on theme configuration to automatically apply changes in real-time
- Refactored theme application logic into a reusable helper method
- Simplified validation configuration (though with some issues noted below)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
app/eventyay/webapp/src/components/ColorPicker.vue |
Changed color input event from @change to @input to emit updates continuously during drag/selection |
app/eventyay/webapp/src/views/admin/config/theme.vue |
Added deep watcher on theme config, extracted _applyThemePreview() helper method, simplified validation config, and removed outdated comments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6ddb416 to
2ff9bed
Compare
|
@prayag78 do you want to test/review this one? |
|
Hi @mariobehling, thank you for the review. However, the UI fix in this PR is straightforward:
These two changes logically enable the real-time preview as requested. I hope the code is clear enough for review on its own. Thank you. |
|
@MohammedAnasNathani |
|
Hi @mariobehling, thanks for asking. I've been working in two environments (local Docker on macOS and GitHub Codespaces) and have run into a persistent WebSocket connection issue. Core Problem: What I've Tried:
I'm confident I can resolve this, but I didn't want to delay the PRs further while I debugged what seems to be an external infrastructure issue. The code fixes themselves are complete and verified through logic and server logs. |
|
@hpdang thanks for giving opportunity to review this PR. Hi @MohammedAnasNathani, thanks for work on this issue. I previously worked on this #1360, where I added a missing CSS regeneration trigger to update the color. During the fixing, I was thinking about the organizer settings, but we can also change colors from the specific event settings. After testing, the current logic correctly regenerates CSS from both settings:
@mariobehling could you please check PR #1360 again? I think both the organizer and event-level CSS regeneration working. |
ArnavBallinCode
left a comment
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.
Hi @mariobehling, thanks for asking. I've been working in two environments (local Docker on macOS and GitHub Codespaces) and have run into a persistent WebSocket connection issue.
Core Problem: The video component fails to establish a WebSocket connection (Error 1006), which prevents the modern admin UI from fully rendering.
What I've Tried:
- Codespaces:
I correctly configuredALLOWED_HOSTS,CSRF_TRUSTED_ORIGINS, and evenCORS_ALLOWED_ORIGINSto match the forwarded Codespace URL. Despite this, the WebSocket connection is still rejected, likely due to a conflict with the Codespaces proxy layer.- Local Docker:
The build process is repeatedly failing at theapt-get installstep with "Hash Sum mismatch" and "File has unexpected size" errors. This points to a temporary issue with the upstream Debian package mirrors, as it persists even after running builds with--no-cache.I'm confident I can resolve this, but I didn't want to delay the PRs further while I debugged what seems to be an external infrastructure issue. The code fixes themselves are complete and verified through logic and server logs.
-
I think you are messing up two things, please start a fresh clean setup using local Docker
-
The WebSocket failure you’re seeing is specific to Codespaces, not the Docker setup. With the current compose file, the WebSocket endpoint runs on the same container and same port as Django
There’s no networking layer that can trigger a 1006 close. Can you restart your system too once.
Also the full stack does run on my system , can you send the log file / screenshot of the logs -local - docker
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| watch: { | ||
| 'config.theme.colors': { | ||
| deep: true, | ||
| handler() { | ||
| this._applyThemePreview() | ||
| } | ||
| } | ||
| }, |
Copilot
AI
Dec 2, 2025
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 deep watcher combined with continuous @input events from the color picker will trigger applyThemeConfig() very frequently while dragging. This function manipulates DOM CSS variables on every call. Consider adding throttling/debouncing to the watcher handler to improve performance during rapid color changes:
import { throttle } from 'lodash' // or implement a simple throttle
export default {
// ...
watch: {
'config.theme.colors': {
deep: true,
handler: throttle(function() {
this._applyThemePreview()
}, 50) // Throttle to max 20 updates/second
}
}
}This would maintain smooth live preview while reducing unnecessary DOM manipulations.
| _applyThemePreview() { | ||
| if (!this.config) return | ||
| const themePayload = { | ||
| colors: {...DEFAULT_COLORS, ...this.config.theme.colors}, | ||
| logo: {...DEFAULT_LOGO, ...this.config.theme.logo}, | ||
| streamOfflineImage: this.config.theme.streamOfflineImage ?? null, | ||
| identicons: {...DEFAULT_IDENTICONS, ...this.config.theme.identicons} | ||
| } | ||
| applyThemeConfig(themePayload) | ||
| }, |
Copilot
AI
Dec 2, 2025
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 watcher will trigger _applyThemePreview() even when color values are invalid (e.g., while typing an incomplete hex code). Since applyThemeConfig() uses the Color() library which throws errors on invalid color strings, this could cause runtime errors during user input.
Consider adding validation before applying the theme:
_applyThemePreview() {
if (!this.config) return
// Don't apply if there are validation errors
if (this.v$.$invalid) return
const themePayload = {
colors: {...DEFAULT_COLORS, ...this.config.theme.colors},
logo: {...DEFAULT_LOGO, ...this.config.theme.logo},
streamOfflineImage: this.config.theme.streamOfflineImage ?? null,
identicons: {...DEFAULT_IDENTICONS, ...this.config.theme.identicons}
}
applyThemeConfig(themePayload)
}Or wrap the applyThemeConfig() call in a try-catch to prevent errors from breaking the UI.
Description
Fixed the issue where live preview of color settings was not working. Previously, the preview only updated after saving.
Issues
Closes #1175
Fix
@changeto@input. This ensures the parent component receives color updates continuously while dragging the color picker, not just when the picker is closed.config.theme.colors. This triggersapplyThemeConfigimmediately whenever colors change, updating the CSS variables in real-time for an instant preview.Type of Change
Summary by Sourcery
Enable real-time theme color preview while editing admin configuration
Bug Fixes:
Enhancements: