Skip to content
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

fix: text color detection bug fixed #1565

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion apps/studio/.env.example
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Only add these if you have valid keys. Otherwise, comment them out.
VITE_SUPABASE_API_URL=
VITE_SUPABASE_ANON_KEY=
VITE_MIXPANEL_TOKEN=
# VITE_MIXPANEL_TOKEN=
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not comment out the example here


# Add your keys here to use Anthropic directly
VITE_ANTHROPIC_API_KEY=
49 changes: 45 additions & 4 deletions apps/studio/src/lib/editor/engine/style/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,51 @@ export class StyleManager {
const newMap = new Map<string, SelectedStyle>();
let newSelectedStyle = null;
for (const selectedEl of selectedElements) {
const styles = {
...selectedEl.styles?.computed,
...selectedEl.styles?.defined,
};
// Debug log to check what styles are coming from the DOM
console.log('Selected element:', selectedEl.tagName);
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't ship logs in production

console.log('Computed styles:', selectedEl.styles?.computed);
console.log('Defined styles:', selectedEl.styles?.defined);

// For text styles like color, prioritize the computed styles from the browser
// which contain the actual rendered values, especially for properties with inheritance
const computedStyles = selectedEl.styles?.computed || {};
const definedStyles = selectedEl.styles?.defined || {};

// Specifically log color values for debugging
if (computedStyles['color']) {
console.log('COMPUTED COLOR:', computedStyles['color']);
}
if (definedStyles['color']) {
console.log('DEFINED COLOR:', definedStyles['color']);
}

// Create a merged styles object, prioritizing computed styles for critical properties
const criticalStyleKeys = [
'color',
'background-color',
'font-family',
'font-size',
'font-weight',
];
const styles: Record<string, string> = { ...definedStyles };

// Ensure critical styles come from computed values when available
for (const key of criticalStyleKeys) {
if (computedStyles[key]) {
styles[key] = computedStyles[key];
}
}

// For non-critical styles, use the defined values if available
for (const key in computedStyles) {
if (!styles[key] && computedStyles[key]) {
styles[key] = computedStyles[key];
}
}

// Log the final merged styles for debugging
console.log('Final merged styles - color:', styles['color']);

const selectedStyle: SelectedStyle = {
styles,
parentRect: selectedEl?.parent?.rect ?? ({} as DOMRect),
Expand Down
5 changes: 5 additions & 0 deletions apps/studio/src/lib/editor/styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ export class SingleStyleImpl implements SingleStyle {
) {}

getValue(styleRecord: Record<string, string>) {
// If this is a color style (key is 'color' or ends with 'Color'), ensure we get the actual value
// This ensures text colors and other color values display correctly in the properties panel
if (this.type === 'color' && styleRecord[this.key]) {
return styleRecord[this.key];
}
return styleRecord[this.key] ?? this.defaultValue;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ const ColorTextInput = memo(
backgroundImage?: string;
}) => {
const inputValue = isFocused ? stagingInputValue : value;
const colorValue = isColorEmpty(inputValue) ? '' : inputValue;
// Don't use isColorEmpty here as it might incorrectly identify valid colors as empty
// If value is a valid hex color (including #000000 for black), show it
const colorValue = inputValue || '';
const displayValue =
backgroundImage && !isBackgroundImageEmpty(backgroundImage)
? backgroundImage
Expand Down Expand Up @@ -95,9 +97,47 @@ const ColorInput = observer(
if (!editorEngine.style.selectedStyle?.styles || isFocused) {
return Color.from(elementStyle.defaultValue);
}
const newValue = elementStyle.getValue(editorEngine.style.selectedStyle?.styles);
return Color.from(newValue);
}, [editorEngine.style.selectedStyle?.styles, elementStyle, isFocused]);

// For color styles specifically, we need to be careful
if (elementStyle.key === 'color') {
// Get the currently selected element
const selectedEl = editorEngine.elements.selected[0];
if (selectedEl) {
const tagName = selectedEl.tagName?.toLowerCase();

// Check if we have a heading or text element
const isTextElement =
tagName === 'p' ||
tagName?.match(/^h[1-6]$/) ||
tagName === 'span' ||
tagName === 'div';

// For text elements, prioritize the actual computed color from the DOM
if (isTextElement && selectedEl.styles?.computed) {
// If we have a computed color, use it
if (selectedEl.styles.computed.color) {
return Color.from(selectedEl.styles.computed.color);
}
}
}
}

// Standard color handling for non-text elements or fallback
const selectedStyles = editorEngine.style.selectedStyle?.styles;

// For color, check the direct style value first
if (elementStyle.key === 'color' && selectedStyles.color) {
return Color.from(selectedStyles.color);
}

// Standard fallback behavior
return Color.from(elementStyle.getValue(selectedStyles));
}, [
editorEngine.style.selectedStyle?.styles,
elementStyle,
isFocused,
editorEngine.elements.selected,
]);

// Update color state when getColor changes
const [color, setColor] = useState(getColor);
Expand Down
9 changes: 9 additions & 0 deletions packages/utility/src/color.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,16 @@ import parseCSSColor from 'parse-css-color';
import { isNearEqual } from './math';

export function isColorEmpty(colorValue: string) {
// Check for common empty color values
if (!colorValue || colorValue === '' || colorValue === 'none' || colorValue === 'transparent') {
return true;
}

// Parse the color to check if it's truly transparent
const color = Color.from(colorValue);

// Only consider it empty if it has zero alpha or exactly equals transparent
// Don't consider black (#000000) or other valid colors as empty
Copy link
Contributor

Choose a reason for hiding this comment

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

We actually don't consider black empty color. You might be seeing #00000000 which is transparent black. The issue seen is actually that there is no set color on the text color instead of it accidentally seeing black as empty. For example, if you set actual black #000000, it'll show up
Screenshot 2025-03-08 at 5 17 24 PM

return color.a === 0 || color.isEqual(Color.transparent);
}

Expand Down