facebook / stylex

StyleX is the styling system for ambitious user interfaces.
https://stylexjs.com
MIT License
8.44k stars 310 forks source link

fix/stylex-content-quotes #782

Closed p0nch000 closed 2 days ago

p0nch000 commented 4 days ago

Fix StyleX content property quotes handling

What changed / motivation

Updated transformValue function to correctly handle CSS content property values:

Previously, StyleX incorrectly wrapped CSS functions like counters() in quotes, breaking their functionality.

Linked PR/Issues

Fixes #771 - Don't wrap content: counters(...) in quotes

Additional Context

Test Coverage

describe('transformValue content property tests', () => {
  test('preserves CSS functions without quotes', () => {
    const functions = [
      ['counters(div, ".")', 'counters(div, ".")'],
      ['counter(chapter)', 'counter(chapter)'],
      ['attr(href)', 'attr(href)'],
      ['url(image.jpg)', 'url(image.jpg)']
    ];
    functions.forEach(([input, expected]) => {
      expect(transformValue('content', input, {})).toBe(expected);
    });
  });

  test('preserves CSS keywords without quotes', () => {
    const keywords = ['normal', 'none', 'open-quote'];
    keywords.forEach(keyword => {
      expect(transformValue('content', keyword, {})).toBe(keyword);
    });
  });

  test('handles mixed content values', () => {
    expect(transformValue('content', 'open-quote counter(chapter)', {}))
      .toBe('open-quote counter(chapter)');
  });
});

Implementation Notes

Pre-flight checklist

p0nch000 commented 4 days ago

@nmn this was my first approach for the content issue, lmk if need to do some fixes :)

nmn commented 4 days ago

@p0nch000 Thanks for the great work. Turns out values for content don't need to be separated by spaces, so we might need to adjust the code a bit.

p0nch000 commented 2 days ago

Heyy took your advice i think is better now, lmk if it needs extra work

p0nch000 commented 2 days ago

thanks for the fast review and the great feedback again! I'll look for my next issue to work on @nmn