css-inline
css-inline copied to clipboard
Stranger6667
WIP: Added additional arg for turning styles into properties
Also, you got to adjust Python bindings to keep the CI green :) The changes should be similar to the changes you've already made
Also, you got to adjust Python bindings to keep the CI green :) The changes should be similar to the changes you've already made
I will give it a shot and ping you if I have any trouble.
Great! I added a few notes :)
Also, the "Features & Configuration" section of the README file would need some adjustments as well to include the new config option :)
For some reason when I open the README.md in my IDE, it only has one line and says "../README.md"... Not sure what that's about
For some reason when I open the README.md in my IDE, it only has one line and says "../README.md"... Not sure what that's about
I think it happens if you open README.md which is inside the css-inline directory because it is a symlink. Opening README.md from the repository root should work fine :) Let me know if the issue persists
@Stranger6667 I tried resolving the merge conflicts to see if I could get this across the finish line, but unfortunately I'm not familiar enough with Rust or the codebase to pull it off. It would also be helpful if, for this implementation, we ensured only attributes that are possible are added (e.g. we don't want to set bgcolor on a div).
@caseyjhol
First of all, I appreciate your effort! I'd be happy to assist with moving it forward.
At this point, the difference between the branches is far too big for a straightforward conflict resolution. Since then, the inlining itself has moved to the serialization phase (i.e. when we write tags & their attributes to the output buffer). The idea is to write those attributes at the very end of the element's serialization instead of writing "style" as done above that line. It will require reworking the style merging process a bit, but it should be more or less similar to the current implementation. We can leave the performance nuances aside for now, but for the record, I think that generalizing the merge behavior via a separate trait is the way to go (i.e. depending on the config option we will just pass a different marker to HtmlSerializer).
I think the first step could be opening a new PR with a few cherry-picked commits e.g. CLI option & config option and then adding new things as mentioned above. I'll try to add some more details to the original issue (or the new PR) and maybe a few more comments to the codebase to make it clearer.
As for ensuring the proper attributes - I am all for it, I think it is a great feature to have! However, it could be better to split it into a separate PR.
Let me know what you think about it!