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 support for replacement of dynamic properties #61

Closed
wants to merge 4 commits into from

Conversation

simonihmig
Copy link
Collaborator

This adds support for having dynamic properties (i.e. bindings) in a snippet, that are replaced with the actual bound value when showing the code snippet.

Take this example to explain the use case:

For the ember-bootstrap demo app I use this addon to show code snippets, as e.g. here for forms: https://www.ember-bootstrap.com/#/components/forms. The code snippet is not entirely static, in this example it showcases three different supported form layouts (vertical, horizontal, inline). The buttons make it possible for the user to dynamically show the form in the different layouts. For this the forms's formLayout property is bound to a property on the controller, that the user is able to change with those buttons.

So far so good, but this means that the code snippet shows this binding (formLayout=formLayout), and this in turn makes the code snippet not work when "blindly" copy-pasted (related: emberjs/ember.js#17544 (comment)). It would be better if the code snippet would show the actual (static) value, like formLayout="horizontal".

This PR makes this possible, by replacing "dynamic properties" with the actual values. This is recorded from the WIP PR using the changes here as a linked addon (see the snippet's @formLayout property):

Kapture 2019-05-26 at 21 54 23

Inspired by: chrislopresto/ember-freestyle#100

Note: this includes #60, so that should be merged first.

@ef4
Copy link
Owner

ef4 commented May 28, 2019

Thanks for working on this @simonihmig.

I think this is a good feature. But I wonder if we can't refactor so that this functionality can be composed from a separate addon, rather than going inside ember-code-snippet. I'm also thinking about #56.

That PR discusses splitting code-snippet into two concerns: one for finding and loading the snippet strings themselves, and another for rendering them with syntax highlighting.

If we did that split, it would give you a great place to insert this dynamic replacement step as well.

One way to imagine the split is that code-snippet becomes a helper that returns the raw snippet information. Something like:

<pre>{{get (code-snippet "sample.js") "source"}}</pre>

would dump the (un-highlighted) source code onto the page. I'm assuming the helper returns a POJO like { source, language, filename }.

But most often people would use a highlighter (which we would split out of ember-code-snippet into a new addon):

<SyntaxHighlighter @code={{code-snippet "sample.js"}} />

Finally, this would let you provide your own helper addon that does the dynamic insertion:

<SyntaxHighlighter @code={{insert-dynamic (code-snippet "sample.js") formLayout=currentLayout }} />

This would separate all three concerns and let people swap different choices into each spot. You could do syntax highlighting of any arbitrary string, no matter where it comes from. You could do dynamic insertion into any string template, whether it comes from ember-code-snippet or not. Etc.

@simonihmig
Copy link
Collaborator Author

simonihmig commented May 28, 2019

Thanks @ef4 for your feedback, which indeed makes a lot of sense!

The interoperability of these different parts is a bit more difficult, or less decoupled, tough:

I'll think about it a bit more, in terms of a nicely composable structure (i.e each part is ideally not coupled to any of the other parts), when I have a bit more time. Also happy for more of your or anyone else's thoughts of course! 😀

@ef4
Copy link
Owner

ef4 commented May 28, 2019

Agreed, I think we would be stabilize the POJO structure as public API.

@simonihmig simonihmig mentioned this pull request May 31, 2019
3 tasks
@simonihmig
Copy link
Collaborator Author

Closing this, as #62 implemented the discussed composability changes, so the replacement feature can be added outside of this addon.

For the record - in case someone else is interested in this - here is my current implementation: ember-bootstrap/ember-bootstrap#819

@simonihmig simonihmig closed this Jun 24, 2019
@simonihmig simonihmig deleted the dynamic-props branch June 24, 2019 18:46
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.

None yet

2 participants