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

Added the possibility for a css property (in addition to the markup property) #86

Closed
wants to merge 3 commits into from

Conversation

anchpop
Copy link

@anchpop anchpop commented Jul 5, 2018

No description provided.

@markfinger
Copy link
Owner

Thanks, but I'm not entirely confident of the benefit of this feature. Can you talk a bit about your use-cases and what you're trying to solve?

@anchpop
Copy link
Author

anchpop commented Jul 6, 2018

@markfinger Sure. I basically only added this because I wanted to get [server side rendering of MaterialUI[(https://material-ui.com/guides/server-rendering/) working and this seemed like the cleanest way. Although I think a better route forward would be to have an extra field called extraData or something, where you could put whatever extra output you need in JSON format. Possibly having something similar for input to the server as well. If I add that to my copy I'll make another PR

@markfinger
Copy link
Owner

Yeah, a more general purpose variable for extra context might be better.

So the styles are being generated in python code and you're passing them down the rendering pipeline? Is there a reason why you need to include the styles in the rendered markup, instead of using your template layer (or equivalent)?

I'm still not sure of the motivating details in the implementation tbh. If you could provide some code it might help. This seems a bit use-case specific, so I'm inclined towards suggesting that you maintain your own fork. This is a tiny and stable lib, so you're not really taking on anything approaching tech debt.

@sassanh
Copy link
Contributor

sassanh commented Jul 16, 2018

@markfinger
I guess this issue should be solved by #87

@markfinger
Copy link
Owner

Yeah, good point, @sassanh.

@anchpop, the work in #87 allows for the response payload from the render server to be passed through the python part of the rendering pipeline. I think this should cover the python side of your usecase

@markfinger markfinger closed this Jul 17, 2018
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