Skip to content

Conversation

@kellyjosephprice
Copy link
Collaborator

@kellyjosephprice kellyjosephprice commented Jun 4, 2024

PR App RM-9849

🧰 Changes

Uses jsx variables for user variables.

The first pass at variables was very simple:

Contact me at: <Variable name='email' />

This replaces with regular old jsx interpolated variables:

Contact me at: {user.email}

Much nicer!

🧬 QA & Testing

@kellyjosephprice kellyjosephprice changed the base branch from next to beta June 4, 2024 23:51
@kellyjosephprice kellyjosephprice marked this pull request as ready for review June 10, 2024 17:57
Copy link
Contributor

@rafegoldberg rafegoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and it's working nicely locally; would approve save for one concern.

export { readmeComponentsTransformer, readmeToMdx, injectComponents, variables };

export default [calloutTransformer, codeTabsTransfromer, embedTransformer, gemojiTransformer];
export default [calloutTransformer, codeTabsTransfromer, embedTransformer, gemojiTransformer, variables];
Copy link
Contributor

@rafegoldberg rafegoldberg Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this export be called variablesTransformer if only for consistency?

Comment on lines 11 to 15
It does not render in code blocks:

```js
const xyz = "<Variable variable="defvar" />";
```
{user.defvar}
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these vars need to be usable within code blocks; I've seen customers use this functionality to construct customized copyable snippets! (It used to be that you'd escape them– something like \<<defvar\>> –if you wanted to show the string literally.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i hate this snap

@kellyjosephprice
Copy link
Collaborator Author

@rafegoldberg approve me?

Copy link
Member

@trishaprile trishaprile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!!

@kellyjosephprice kellyjosephprice merged commit 3ed86e3 into beta Jun 13, 2024
@kellyjosephprice kellyjosephprice deleted the kp/variables branch June 13, 2024 20:59
rafegoldberg pushed a commit that referenced this pull request Jun 13, 2024
## Version 6.75.0-beta.57

### ✨ New & Improved

* jsx variables ([#899](#899)) ([3ed86e3](3ed86e3))

<!--SKIP CI-->
@rafegoldberg
Copy link
Contributor

This PR was released!

🚀 Changes included in v6.75.0-beta.57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants