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

Use tailwind typography plugin by default #3593

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

minimav
Copy link
Contributor

@minimav minimav commented Jun 29, 2024

This PR addresses #1529 to make the HTML component display tags correctly by default via the Tailwind typography plugin as suggested on the issue itself and using the prose class as mentioned here.

Example of this working:

Screenshot 2024-06-29 at 15 28 19

via this:

Screenshot 2024-06-29 at 15 28 37

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md file?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • New feature (non-breaking change which adds functionality)

New Feature Submission:

  • Does your submission pass the tests?
  • Have you linted your code locally prior to submission?

Changes To Core Features:

  • Have you successfully ran tests with your changes locally?

@Lendemor
Copy link
Collaborator

Lendemor commented Jul 2, 2024

I don't have an issue about the code itself, I'm just not sure if we should have the tailwindcss/typography as a default for every project, rather than enabling it when rx.html is used.

@masenf
Copy link
Collaborator

masenf commented Jul 2, 2024

@Lendemor is there a perceived downside to bringing in the typography plugin for every project? Slightly large download size perhaps, but i think in deploy/prod mode the extra classes will be pruned out if prose is not actually used anywhere.

@Lendemor
Copy link
Collaborator

Lendemor commented Jul 4, 2024

@Lendemor is there a perceived downside to bringing in the typography plugin for every project? Slightly large download size perhaps, but i think in deploy/prod mode the extra classes will be pruned out if prose is not actually used anywhere.

Probably nothing big, I just remembered some users disabling tailwind in their projects (we had it included as default before, then we made it optional at some point) because it was conflicting with some others stuff they were using I think.

If we enable it forcefully again, it might break these users apps? I'm not sure how many users this is tho, so maybe it's not a big deal.

@masenf
Copy link
Collaborator

masenf commented Jul 8, 2024

If we enable it forcefully again

If user sets tailwind=None in their config, then they will not get the plugins either, so i think it's safe

@adhami3310 adhami3310 self-requested a review September 24, 2024 00:44
Copy link
Member

@adhami3310 adhami3310 left a comment

Choose a reason for hiding this comment

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

Sorry for sleeping on it 😅

@adhami3310 adhami3310 merged commit 2c4310d into reflex-dev:main Sep 24, 2024
32 checks passed
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.

4 participants