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

disable prose by default for rx.html #4001

Merged
merged 5 commits into from
Sep 26, 2024
Merged

Conversation

adhami3310
Copy link
Member

No description provided.

Lendemor
Lendemor previously approved these changes Sep 25, 2024
Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

what is the motivation for this change?

@adhami3310
Copy link
Member Author

what is the motivation for this change?

some people (carlos) rely on existing html code that looks very different now, so we're preserving existing behavior.

additionally, it reflects the behavior of doing rx.el.h1 for example, which is unstyled by default.

@masenf
Copy link
Collaborator

masenf commented Sep 25, 2024

so this partly reverts the change brought in by #3593

but looking at that PR closer i see that it doesnt do anything with rx.markdown but the original bug it was supposed to be fixing specifically mentions markdown and html being affected (maybe markdown got fixed since then).

i guess what im getting at is why not just revert that entire PR? this new styled prop is confusingly named IMO and not particularly discoverable.

@adhami3310
Copy link
Member Author

not just revert that entire PR? this new styled prop is confusingly named IMO and not particularly discoverable

good question! i pose that some might find it convenient to bring back that property, but also, they could do class_name="prose" to do the same behavior, making this essentially uesless so 🤷 .

@adhami3310 adhami3310 merged commit 54c7b5a into main Sep 26, 2024
39 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.

3 participants