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 proxy variables' HTML for ALKiln >=5.9.0 2-column Story Table #824

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

plocket
Copy link
Collaborator

@plocket plocket commented Feb 14, 2024

Proxy variables are generic objects and index variables. I don't think this needs to be added anywhere else.

Closes #823

Proxy variables are generic objects and index variables
<div data-variable="${ encode_name(str( user_info().variable )) }" id="trigger" aria-hidden="true" style="display: none;"></div>
Copy link
Member

Choose a reason for hiding this comment

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

Could we reasonably combine this with the template above, or are people likely to want one but not both?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a good question. I want people to use the new one, not both. There's no reason for new tests to use the old one. Assembly Line probably needs to keep it around for now to keep people's old tests working, but I want new Story Tables to avoid it if possible.

I also I think keeping that old id ("trigger") around with the new stuff would only add to confusion as it's not relevant anymore. It's hard enough in both our internal and external documentation to explain what's going on with Story Tables - decoding and guessing variable names, etc.

That said, if you feel it'll be detrimental to AssemblyLine, ALKiln can either do as you describe or make an exception to handle interviews that do so.

Copy link
Member

@nonprofittechy nonprofittechy left a comment

Choose a reason for hiding this comment

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

I'll trust your judgment on the separate templates. Please feel free to merge once tests are passing

@plocket plocket merged commit a121bc6 into main Feb 21, 2024
5 checks passed
@plocket plocket deleted the 823_alkiln_proxy_vars_html branch February 21, 2024 19:42
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.

Update ALKiln post: to add new 2-column Story Table HTML element
2 participants