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

feat: theme & content now a plugin #180

Merged
merged 22 commits into from
Nov 29, 2023
Merged

feat: theme & content now a plugin #180

merged 22 commits into from
Nov 29, 2023

Conversation

Ademsk1
Copy link
Contributor

@Ademsk1 Ademsk1 commented Nov 27, 2023

Fixes #178

Plugin imported via require('@nearform/quantum/plugin'). Tested on a separate repo using yalc and results seemed good, required me to pass in our package to content. Pushing our package into content doesn't seem possible. Here were my findings

UPDATE
Was able to push to the config easily. It seems the config() actually returns the consuming applications object by reference, and so was able to modify it as such.

~~1. Adding the content key into the plugin was unsuccessful for a variety of different paths (I assumed there may be some dynamic imports necessary, tried them all and no luck).
2. Several of the big component libraries using Tailwind (Flowbite, Preline) require you to add in the package as content manually.
3. The Plugin page does not mention content being modifiable anywhere, however presets does. It first seemed like a better avenue to go down, as the docs quote about presets:

"This can be very useful for teams that manage multiple Tailwind projects for the same brand where they want a single source of truth for colors, fonts, and other common customizations."

However, if one scrolls down to to the merging logic section , you can see that the consumers content would overwrite our own. I've raised a discussion about other avenues to resolve this here. ~~

Copy link

@review-bot-for-github review-bot-for-github bot left a comment

Choose a reason for hiding this comment

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

Looks good! No comments.

@Ademsk1 Ademsk1 requested a review from nherment November 27, 2023 15:30
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@Ademsk1 Ademsk1 requested review from nherment and simoneb November 28, 2023 12:02
@Ademsk1 Ademsk1 changed the title feat: theme now a plugin feat: theme & content now a plugin Nov 28, 2023
README.md Outdated Show resolved Hide resolved
src/plugin.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@nstolpe nstolpe left a comment

Choose a reason for hiding this comment

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

This looks good. I found one unimplemented test that could use some logic (or maybe removal?) and a few comments that look like they could be removed.

__tests__/plugin.test.ts Outdated Show resolved Hide resolved
__tests__/plugin.test.ts Outdated Show resolved Hide resolved
__tests__/plugin.test.ts Outdated Show resolved Hide resolved
@Ademsk1 Ademsk1 requested a review from nstolpe November 29, 2023 10:56
Copy link
Contributor

@nstolpe nstolpe left a comment

Choose a reason for hiding this comment

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

👍

@Ademsk1 Ademsk1 merged commit b9eed4e into main Nov 29, 2023
3 checks passed
@Ademsk1 Ademsk1 deleted the feat/178_plugin branch November 29, 2023 14:34
@Ademsk1 Ademsk1 mentioned this pull request Nov 30, 2023
2 tasks
@github-actions github-actions bot mentioned this pull request Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Tailwind's plugin mechanism
4 participants