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

CSS style sheets for embedded form not inherited from the parent document #8347

Closed
twschiller opened this issue Apr 25, 2024 · 3 comments · Fixed by #8353
Closed

CSS style sheets for embedded form not inherited from the parent document #8347

twschiller opened this issue Apr 25, 2024 · 3 comments · Fixed by #8353
Assignees
Labels

Comments

@twschiller
Copy link
Contributor

twschiller commented Apr 25, 2024

Describe the bug

To Reproduce

Steps to reproduce the behavior:

  1. Create a document with a theme CSS, e.g., https://pixiebrix-public-stylesheets.s3.us-east-2.amazonaws.com/pixiebrix/snippet-manager.css?versionId=bdX2jLRU2EH54uG3uLEOm28vuOl0Ny5R
  2. Add a form

I also created a mod here we can use for testing: @pixies/testing/panel-theme

Actual behavior

Theme CSS is not applied to the form

Expected behavior

Theme should be applied to the form

Screenshots/Loom

image

image

Workaround

  • Repeat the stylesheet in the Theme for the form

Desktop (please complete the following information):

  • Extension Version: 1.8.13-beta.5-mv3

Discussion

  • I think CSS behavior would be a good use of Playwright visual snapshot testing, so we can detect unintentional changes in rendering

Testing

  • I created a mod here we can use for testing: @pixies/testing/panel-theme

Investigation

@mnholtz mnholtz self-assigned this Apr 26, 2024
@twschiller
Copy link
Contributor Author

@mnholtz if we don't want to use VRT for testing, could potentially use CSS classes with isVisible assertion to check if the CSS was applied correctly? (By playing around with display: none)?

@mnholtz
Copy link
Collaborator

mnholtz commented Apr 26, 2024

(By playing around with display: none)?

That's a great idea, I'll probably take that approach for now. Want to have a dedicated VRT discussion/ticket.

@twschiller
Copy link
Contributor Author

twschiller commented Apr 26, 2024

@mnholtz Actually, it looks like Playwright has support for asserting computed styles: https://playwright.dev/docs/api/class-locatorassertions#locator-assertions-to-have-css

So you can use @pixies/testing/panel-theme as-is and assert the font color

mnholtz added a commit that referenced this issue Apr 29, 2024
…8353)

* introduce empty sidebarPanelTheme spec file

* introduce sidebar theme test

* add steps for opening the sidebar

* finish failing test with green assertions

* replace StylesheetsContext

* replace StylesheetsContext provider and hooks usage

* fix lint errors and add StylesheetsContext to strictnull

* replace ephemeralformcontent logic

* fix strict null error
twschiller pushed a commit that referenced this issue Apr 30, 2024
…8353)

* introduce empty sidebarPanelTheme spec file

* introduce sidebar theme test

* add steps for opening the sidebar

* finish failing test with green assertions

* replace StylesheetsContext

* replace StylesheetsContext provider and hooks usage

* fix lint errors and add StylesheetsContext to strictnull

* replace ephemeralformcontent logic

* fix strict null error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants