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

Fix html2ft edge cases, group custom attrs, add tests #581

Merged
merged 4 commits into from
Nov 22, 2024

Conversation

callmephilip
Copy link
Contributor


name: Pull Request
about: Propose changes to the codebase
title: '[PR] '
labels: ''
assignees: ''


Related Issue

Current issues with html2f implementation

<div x-transition:enter.scale.80 x-transition:leave.scale.90>hello</div> incorrectly converts to Div('hello', **{'x-transition:enter.scale.80':''}, **{'x-transition:leave.scale.90':''})

<div id="1">hello</div> incorrectly converts to Div('hello', id='1') with attrs1st=True

(Not a bug but inconvenience):

Div('Hello 👋', x_show='open', **{'x-transition:enter':'transition duration-300'}, **{'x-transition:enter-start':'opacity-0 scale-90'}) <- custom attributes spread around multiple dictionaries as opposed to being grouped into 1 dictionary, e.g.

Div('Hello 👋', x_show='open', **{'x-transition:enter':'transition duration-300', 'x-transition:enter-start':'opacity-0 scale-90'})

Proposed Changes
This PR addresses the issues mentioned above ⬆️ and adds tests for htm2ft function

Types of changes
What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)

Copy link

gitnotebooks bot commented Nov 19, 2024

Found 1 changed notebook. Review the changes at https://app.gitnotebooks.com/AnswerDotAI/fasthtml/pull/581

@jph00
Copy link
Contributor

jph00 commented Nov 22, 2024

Thanks @callmephilip ! Can you please run nbdev_export to fix CI?

@callmephilip
Copy link
Contributor Author

Thanks @callmephilip ! Can you please run nbdev_export to fix CI?

I tried the following multiple times:

nbdev_export && nbdev_clean && nbdev_trust

Still no luck. It seems like something has happened to _modidx

@jph00
Copy link
Contributor

jph00 commented Nov 22, 2024 via email

@callmephilip
Copy link
Contributor Author

nuked project directory and did a fresh checkout. looks good now

@jph00 jph00 merged commit 1e40a53 into AnswerDotAI:main Nov 22, 2024
1 check passed
@jph00
Copy link
Contributor

jph00 commented Nov 22, 2024

Many thanks :)

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.

2 participants