You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Built on top of #1828; this doesn't belong in that PR, but also that PR is currently subtly broken, so if we accept this change then we should merge this PR instead.
Unfortunately, this PR makes squiggle-components harder to adopt for those who don't use Tailwind. But easier for those who do use Tailwind, including our team.
This is mostly Tailwind's fault; it doesn't provide a good solution for how to scope its preflight styles.
I got lucky with this PR that the solution for Docusaurus exists: facebook/docusaurus#2961 (comment), and it works pretty well. Though I don't like that it loads an extra CSS file from the CDN. But we'll probably migrate from Docusaurus to Nextra (#1801) before this causes us any problems.
In the future, if we want to simplify the adoption of squiggle-components by non-Tailwind teams, we'll have to do something like this:
Write a detailed instruction on how to scope Tailwind's preflight (this requires copy-paste, like I did in the previous version of squiggle-components, or some non-standard postcss plugin).
Alternatively, we could bundle a scoped preflight in @quri/ui, with a default name.
Ask consumers to configure Tailwind to build our styles; they'll have to analyze both @quri/ui and @quri/squiggle-components for Tailwind classes, and if we ever split our packages further, consumers will have to update their tailwind.config.js file.
In addition to that, our modals and tooltips use portals, so they need to be scoped too; in the previous version of @quri/ui, this was done by tailwindSelector prop; in the future, this could be achieved by the top-level <TailwindContext.Provider value={{selector: "squiggle"}}>, which would be picked up by @quri/ui components and injected into portals. Then we'd have to ask squiggle-components consumers to put that provider somewhere at the top level of their app. (It's not possible to pass tailwindSelector explicitly, because modals are rendered by squiggle-components, so the necessary selector class must be passed through the provider.)
To clarify, not everyone has to include a scoped preflight for components to work. But Tailwind's non-scoped preflight is pretty aggressive (i.e., it resets h1/h2/h3 styles), so it would usually break user's other styles. In case of Docusaurus, the solution that I linked above injects Tailwind's unscoped preflight before Docusaurus global styles, which makes them compatible. In other setups, this might be harder/impossible to do.
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
In addition to that, our modals and tooltips use portals, so they need to be scoped too; in the previous version of @quri/ui, this was done by tailwindSelector prop; in the future, this could be achieved by the top-level <TailwindContext.Provider value={{selector: "squiggle"}}>, which would be picked up by @quri/ui components and injected into portals. Then we'd have to ask squiggle-components consumers to put that provider somewhere at the top level of their app. (It's not possible to pass tailwindSelector explicitly, because modals are rendered by squiggle-components, so the necessary selector class must be passed through the provider.)
This part bothered me, so I implemented <TailwindProvider> and rewrote components README file to explain how to use it.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1892.
Built on top of #1828; this doesn't belong in that PR, but also that PR is currently subtly broken, so if we accept this change then we should merge this PR instead.
Unfortunately, this PR makes squiggle-components harder to adopt for those who don't use Tailwind. But easier for those who do use Tailwind, including our team.
This is mostly Tailwind's fault; it doesn't provide a good solution for how to scope its preflight styles.
I got lucky with this PR that the solution for Docusaurus exists: facebook/docusaurus#2961 (comment), and it works pretty well. Though I don't like that it loads an extra CSS file from the CDN. But we'll probably migrate from Docusaurus to Nextra (#1801) before this causes us any problems.
In the future, if we want to simplify the adoption of squiggle-components by non-Tailwind teams, we'll have to do something like this:
@quri/ui, with a default name.@quri/uiand@quri/squiggle-componentsfor Tailwind classes, and if we ever split our packages further, consumers will have to update their tailwind.config.js file.@quri/ui, this was done bytailwindSelectorprop; in the future, this could be achieved by the top-level<TailwindContext.Provider value={{selector: "squiggle"}}>, which would be picked up by@quri/uicomponents and injected into portals. Then we'd have to ask squiggle-components consumers to put that provider somewhere at the top level of their app. (It's not possible to passtailwindSelectorexplicitly, because modals are rendered by squiggle-components, so the necessary selector class must be passed through the provider.)To clarify, not everyone has to include a scoped preflight for components to work. But Tailwind's non-scoped preflight is pretty aggressive (i.e., it resets h1/h2/h3 styles), so it would usually break user's other styles. In case of Docusaurus, the solution that I linked above injects Tailwind's unscoped preflight before Docusaurus global styles, which makes them compatible. In other setups, this might be harder/impossible to do.