Skip to content

Conversation

@markconroy
Copy link
Member

Closes #51

@markconroy
Copy link
Member Author

I think the test failure is a red-herring. If not, it's got nothing to do with this PR, so this PR should be ready to merge.

@Adnan-cds
Copy link
Contributor

Thanks a lot Mark. Nothing to complain about the form config at first glance. I will test it and get back to you.

Finn, would you prefer this demo form config to remain in its own module or go into the example module you were thinking of recently. @finnlewis

@andybroomfield
Copy link
Contributor

Not seeing any module code here, is it just the example form?
If so, this is something that perhaps belongs in Localgov Examples

@markconroy
Copy link
Member Author

@andybroomfield this is just for the demo form, yes.

I didn't know we had an examples module and I'm not sure others would think of looking for that to see an example of a form. I'd prefer to have it here if that's okay.

@andybroomfield
Copy link
Contributor

What do others think? I guess we really need to promote the localgov_examples module?
It feels odd putting a non-functional module here, but if we do we should clearly label the sub module as an example, as others might expect it provides a fully functional desicion tree element rather than demoing how to use the built in webform conditionals.

@Adnan-cds
Copy link
Contributor

Sorry Andy, I meant to respond yesterday but got distracted by the LocalGov Outpost integration session and just forgot :( I also wasn't aware that there's a localgov_examples module! I don't mind the Decision tree demo module going in localgov_examples while the README of localgov_forms includes a pointer to it. We may even create an example Webform collection submodule within localgov_examples and put all our demo Webforms in it. Perhaps this is something for Finn the think about. Copying @finnlewis

@markconroy
Copy link
Member Author

@andybroomfield The webform this creates is called Demo Decision Tree: Find the Perfect Playlist. That should be clear enough to illustrate that it's just a demo and not a fully functioning smart answer engine.

@andybroomfield
Copy link
Contributor

@markconroy :-) I was looking at the machine name, can that match? localgov_forms_demo_descion_tree

@markconroy
Copy link
Member Author

@andybroomfield machine name updated to localgov_forms_demo_descion_tree

@finnlewis
Copy link
Member

Discussing in Merge Monday.

@ekes is happy with including config in example modules, see localgovdrupal/localgov#644 (comment)

@Adnan-cds suggested that maybe we re-name this module to add more examples? If we also add a Liberty Create example, we will probably want a README with some docs.

localgov_forms_examples

If we think we're going to have more webform examples to share, perhaps that makes sense.

Thoughts @markconroy ?

@markconroy
Copy link
Member Author

Perhaps a submodule of localgov_forms_examples so you can have a module per form (so we don't need to turn them all one in one go).

Any thoughts on how we let people who install localgov_forms know there's some examples of forms in another module somewhere else?

@ekes
Copy link
Member

ekes commented Nov 27, 2023

As webform ships with three example modules, and the developer likes adding things, including a 'plug-ins' section that includes some example modules, I assumed the webform examples would appear somewhere. Nope. They're under 'Webform examples' in 'Extend'.

It's maybe a governance question, but should we make a 'LocalGov Drupal examples' section. Include this in it. But also include (in include-dev) localgovdrupal/localgov_examples and those list in there too. As it'd be in --with-dev (default unless you're installing to prod) it would become to be a place people look?

@finnlewis
Copy link
Member

Discussing in Merge Monday.

@ekes is happy to merge this as is and follow up with a pull request to change the name of the package group, such that in the extend modules page, this module will be listed as 'Localgov Drupal Examples' .

Copy link
Contributor

@Adnan-cds Adnan-cds left a comment

Choose a reason for hiding this comment

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

Thanks.

@Adnan-cds Adnan-cds merged commit 842c1c2 into 1.x Nov 27, 2023
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 Demo Decision Tree Webform

6 participants