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

Move the ALKiln trigger variable code out of the styled package #355

Closed
plocket opened this issue Dec 28, 2021 · 8 comments · Fixed by #742
Closed

Move the ALKiln trigger variable code out of the styled package #355

plocket opened this issue Dec 28, 2021 · 8 comments · Fixed by #742
Assignees
Labels
bite size upstream/core Track issues that would be fixed upstream before we're ready to create an upstream issue

Comments

@plocket
Copy link
Collaborator

plocket commented Dec 28, 2021

post: |
<div data-variable="${ encode_name(str( user_info().variable )) }" id="trigger" aria-hidden="true" style="display: none;"></div>

This is currently in al_visual.yml, which is excluded from al_package_unstyled.yml. That means that developers that use that unstyled version have to do extra work to set up for ALKiln testing. In addition, this doesn't do anything visual to the interviews.

Is there a better place to put this code?

@plocket plocket changed the title Move the trigger variable code out of the styled package Move the ALKiln trigger variable code out of the styled package Dec 28, 2021
@nonprofittechy
Copy link
Member

nonprofittechy commented Dec 29, 2021

Not sure of other options to place the code, but I feel pretty sure that the al_package_unstyled.yml shouldn't override any screen parts, especially since default screen parts takes precedence over configuration screen parts and interview metadata tag. Edit: to explain--even though this doesn't show something visual, it could easily remove something visual that the author didn't expect.

One option would just be to clearly document this as something who wants to use the unstyled code will need to manually add. We could also move this snippet of HTML into a template so it's better encapsulated and can be centrally updated for both the styled/unstyled variations.

@plocket
Copy link
Collaborator Author

plocket commented Dec 30, 2021

I like the idea of a template. What file would that live in?

Regarding documentation, it's there already 👍 . [That said, that alone doesn't seem like a complete solution. If someone can figure out a better way to do this that's more flexible than screen parts, I'd be super into that.]

@nonprofittechy
Copy link
Member

I think the template could go in al_code.yml. No strong feelings though--just something that is already included in assembly_line_unstyled.yml would be best.

@nonprofittechy nonprofittechy added the wontfix This will not be worked on label Dec 15, 2022
@nonprofittechy nonprofittechy closed this as not planned Won't fix, can't repro, duplicate, stale Dec 15, 2022
@BryceStevenWilley
Copy link
Contributor

IMO this should remain open. Switching to a template is pretty straight forward, shouldn't break anything existing, and would make using it with unstyled a bit easier. The only thing that makes it take more than 5 minutes is aligning docs, but it's still a small task.

@nonprofittechy
Copy link
Member

Eh, that's fine. I think this is describing a vanishingly small use case, but I'm fine to let someone else give it a try.

@BryceStevenWilley BryceStevenWilley self-assigned this Dec 16, 2022
@BryceStevenWilley BryceStevenWilley removed the wontfix This will not be worked on label May 16, 2023
@nonprofittechy
Copy link
Member

@plocket can we describe why the trigger variable is still needed, and are there any other upstream options we should push for?

@nonprofittechy nonprofittechy added upstream/core Track issues that would be fixed upstream before we're ready to create an upstream issue bite size labels Aug 22, 2023
BryceStevenWilley added a commit that referenced this issue Aug 22, 2023
Makes it easier to be reused in unstyled interviews, while keeping all styled
and unstyled interviews the same.

Fixes #355.
@BryceStevenWilley
Copy link
Contributor

I made a quick patch for this. Sorry, I've probably spent more time responding to the issue than it took to just change it.

The trigger variable is needed because docassemble's encoding of field ids and names on generic objects still uses x. So on a generic variable screen, there's nothing in the HTML itself that we can use to determine what x is for x.name.first, or whatever. I don't think Jonathon would want an upstream fix here; the slack threads talking about this are archived.

We have additional things that we might add to this template as well, as in SuffolkLITLab/ALKiln#582. I don't think it's something that we can push upstream.

@plocket
Copy link
Collaborator Author

plocket commented Aug 24, 2023

Jonathan at some recent point mentioned that he would consider changing some of the stuff to do with getField() and its friends to allow js on the page to be able to more easily access fields and values with just their variable names. [That would get rid of the need for any of this stuff.] I haven't seen those changes yet, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bite size upstream/core Track issues that would be fixed upstream before we're ready to create an upstream issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants