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

add .inner_html() builder method #378

Merged
merged 6 commits into from
Jun 18, 2022
Merged

Conversation

alsuren
Copy link
Contributor

@alsuren alsuren commented Mar 7, 2022

This is useful for injecting SVGs into the dom.

I have not tested this with SSR or hydration, because I'm not using them in my current project.

Screenshot 2022-03-07 at 11 56 21

@codecov
Copy link

codecov bot commented Mar 7, 2022

Codecov Report

Merging #378 (a1b3abb) into master (95a7e14) will decrease coverage by 0.15%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
- Coverage   64.99%   64.84%   -0.16%     
==========================================
  Files          52       52              
  Lines        8305     8325      +20     
==========================================
  Hits         5398     5398              
- Misses       2907     2927      +20     
Impacted Files Coverage Δ
packages/sycamore/src/builder.rs 21.93% <0.00%> (-1.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95a7e14...a1b3abb. Read the comment docs.

@alsuren
Copy link
Contributor Author

alsuren commented Mar 7, 2022

I guess this relates to #288 - we have tried a few things in 0.7-land, like:

  • building svgs by hand using the builder api (typically a bad idea, because you end up with an svg that it impossible to view or edit outside of rust-land)

  • falling back to the view!{} macro (okay, but typically forces you to add another layer of div or span that you don't really want).

  • encoding the svg as a data url:

    let icon = format!(
    "data:image/svg+xml;charset=UTF-8,{}",
    utf8_percent_encode(source.icon(), ASCII_SET)
    );
    img().attr("src", icon).build()

Since this patch is tiny, we might backport it to 0.7 as well, so I can see how it feels when used with our existing codebase. That way, we give an experience report.

@alsuren alsuren mentioned this pull request Mar 7, 2022
Ben-PH added a commit to Ben-PH/sycamore that referenced this pull request Mar 7, 2022
@lukechu10 lukechu10 linked an issue Mar 8, 2022 that may be closed by this pull request
@mc1098
Copy link
Contributor

mc1098 commented Jun 8, 2022

Is it just the function name change that is blocking this from being merged? :)

@lukechu10
Copy link
Member

Sorry haven't taken a look in this in a while. It's that and also I'd like to keep the example the way it is right now to be consistent with the non-builder equivalent. I can push a commit on my side to fix that and merge this soon.

@lukechu10 lukechu10 enabled auto-merge (squash) June 18, 2022 21:32
@lukechu10 lukechu10 disabled auto-merge June 18, 2022 21:32
@lukechu10 lukechu10 enabled auto-merge (squash) June 18, 2022 21:32
@lukechu10 lukechu10 merged commit adc2abb into sycamore-rs:master Jun 18, 2022
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.

Add dangerously_set_inner_html support to builder API
3 participants