Skip to content

Fix text color handling in custom themes #519

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 34 additions & 7 deletions src/components/DialogBox/Themes/ApplyThemes.vue
Original file line number Diff line number Diff line change
Expand Up @@ -181,16 +181,43 @@ function changeTheme(theme: string) {
}

function changeCustomTheme(e: InputEvent) {
const customTheme = customThemesList[(e.target as HTMLInputElement).name as keyof typeof customThemesList];
const target = e.target as HTMLInputElement;
const name = target.name as keyof typeof customThemesList;
const value = target.value;

// Update the UI model
const customTheme = customThemesList[name];
if (customTheme) {
customTheme.color = (e.target as HTMLInputElement).value;
customTheme.color = value;

// Apply to all referenced CSS properties
customTheme.ref.forEach((property: string) => {
themeOptions['Custom Theme'][property] = (e.target as HTMLInputElement).value
})
themeOptions['Custom Theme'][property] = value;
});

// Extra handling for text color
if (name === 'Text') {
// Ensure the --text variable is always set
themeOptions['Custom Theme']['--text'] = value;
}
}

// Save back to the model
customThemesList[name] = customTheme;

// Apply the theme
updateThemeForStyle('Custom Theme');
updateBG();

// Force text elements to update
if (window.globalScope) {
setTimeout(() => {
// Re-render canvas with new colors
if (window.globalScope && window.globalScope.renderCanvas) {
window.globalScope.renderCanvas();
}
}, 10);
}
customThemesList[(e.target as HTMLInputElement).name as keyof typeof customThemesList] = customTheme
updateThemeForStyle('Custom Theme')
updateBG()
}

function applyTheme() {
Expand Down
4 changes: 2 additions & 2 deletions v0/src/simulator/src/themer/customThemeAbstraction.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ export const CreateAbstraction = (themeOptions) => {
ref: ['--canvas-stroke'],
},
Text: {
color: themeOptions['--text-lite'],
color: themeOptions['--text'],
description: 'text color',
ref: ['--text-lite', '--text-panel', '--text-dark'],
ref: ['--text', '--text-panel', '--input-text'],
},
Borders: {
color: themeOptions['--br-secondary'],
Expand Down
23 changes: 20 additions & 3 deletions v0/src/simulator/src/themer/themer.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,29 @@ export let colors = getCanvasColors()
*/
export function updateThemeForStyle(themeName) {
const selectedTheme = themeOptions[themeName]
if (selectedTheme === undefined) return
const html = document.getElementsByTagName('html')[0]
Object.keys(selectedTheme).forEach((property, i) => {
if (!selectedTheme) return

const html = document.documentElement

// Set all theme properties
Object.keys(selectedTheme).forEach((property) => {
html.style.setProperty(property, selectedTheme[property])
})

// Ensure text color is properly set for custom themes
if (themeName === 'Custom Theme' && selectedTheme['--text']) {
html.style.setProperty('--text', selectedTheme['--text'])
}

// Update colors object with current CSS variables
colors = getCanvasColors()

// Force canvas refresh to apply new colors
if (window.globalScope && window.globalScope.renderCanvas) {
setTimeout(() => {
window.globalScope.renderCanvas()
}, 10)
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion v1/src/simulator/src/themer/customThemeAbstraction.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export const CreateAbstraction = (themeOptions) => {
Text: {
color: themeOptions['--text-lite'],
description: 'text color',
ref: ['--text-lite', '--text-panel', '--text-dark'],
ref: ['--text-lite', '--text-panel', '--text-dark', '--text'],
},
Borders: {
color: themeOptions['--br-secondary'],
Expand Down
25 changes: 22 additions & 3 deletions v1/src/simulator/src/themer/themer.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ const getCanvasColors = () => {
colors['canvas_fill'] = getComputedStyle(
document.documentElement
).getPropertyValue('--canvas-fill')
colors['text'] = getComputedStyle(
document.documentElement
).getPropertyValue('--text')
Comment on lines +72 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove duplicate assignment to colors['text']

The colors['text'] property is already assigned at lines 51-53 with the same value. This duplicate assignment should be removed to avoid redundancy and potential confusion.

- colors['text'] = getComputedStyle(
-     document.documentElement
- ).getPropertyValue('--text')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
colors['text'] = getComputedStyle(
document.documentElement
).getPropertyValue('--text')

return colors
}

Expand All @@ -84,12 +87,28 @@ export let colors = getCanvasColors()
*/
export function updateThemeForStyle(themeName) {
const selectedTheme = themeOptions[themeName]
if (selectedTheme === undefined) return
const html = document.getElementsByTagName('html')[0]
Object.keys(selectedTheme).forEach((property, i) => {
if (!selectedTheme) return

const html = document.documentElement

// Set all theme properties
Object.keys(selectedTheme).forEach((property) => {
html.style.setProperty(property, selectedTheme[property])
})

// Special handling for text color in all themes
if (selectedTheme['--text']) {
// Ensure text color is explicitly set
html.style.setProperty('--text', selectedTheme['--text'])
}

// Update colors object for canvas elements
colors = getCanvasColors()

// Force a redraw of the canvas to apply the new colors
if (window.globalScope && window.globalScope.renderCanvas) {
setTimeout(() => window.globalScope.renderCanvas(), 10)
}
}

/**
Expand Down