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 form builder test and add collapsible sub forms #161

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Rahien
Copy link
Contributor

@Rahien Rahien commented Nov 28, 2023

Adds collapsible subforms, also adds the form builder form for testing

image

Forms can be made collapsible by adding form:isCollapsible true (new predicate).

A collapsible form can also receive a path that specifies where to find the label/value to show when the form is collapsed. This is done using for example: form:collapsedLabelPath ( sh:name ) ; in this case, the form will follow the specified path in its source graph and take the FIRST resulting value and render it when it is closed.

# Subform: ext:formListingFormItem
##########################################################
ext:formListingFormItem a form:SubForm;
sh:name "Listing";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a change from how the actual form builder organizes its forms. By putting the name at the form level, the controls are nicely wrapped around the title of the form. Otherwise the title (of the propertygroup as configured in the real form builder) appears below the controls and looks out of place.

@@ -34,7 +55,7 @@
</AuToolbar>
{{/if}}

<div class="au-o-flow">
<div class="au-o-flow {{if this.collapsed 'au-u-hidden'}}">
Copy link
Contributor Author

@Rahien Rahien Nov 28, 2023

Choose a reason for hiding this comment

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

using au-u-hidden here because it seems that rendering it conditionally using #if or #unless breaks the children of a propertygroup

##########################################################
ext:formListingFormItem a form:SubForm;
sh:name "Listing";
form:isCollapsible true;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably me but I thought that the predicate form:isCollapsible would define if it is possible to collapse but I see that it is used for the state of it?
When testing in the formbuilder dummy app I also can't see a difference when putting the predicate on false or true. When showing the value of the isCollapsible in the UI it shows the 1 and 0 when changing but nothing is collapsed or uncollapsed.
image

What I assume we wanted to achieve is that we could define if the collapse toggle is possible on the item + when collapsing is possible that we can add a start state for it?

If i' wrong than i'm wrong 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey Jonas, you're absolutely right, thanks! I didn't check the value of the form:isCollapsible predicate and just assumed that if it existed, it meant that they wanted it collapsible. With the latest commit I now expect the value to be '1' which is how a boolean true seems to be handled in rdflib.
I also made the listings not collapsible by default.

You got it right that form:isCollapsible was meant to signify whether collapsing is possible, it's not about the state. I wasn't planning on adding a predicate to signify whether a form is collapsed or not by default. Do you think it's worth having? Would it be something like form:collapsedByDefault true; then?

Copy link
Contributor

@JonasVanHoof JonasVanHoof Dec 4, 2023

Choose a reason for hiding this comment

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

The predicate now does what it's should be doing great! Thanks 🤩

That's a good one im not how it should be implemented.. In my head: if you have a listing with fields the fields in that listing would be collapsed on init but that's not something that you could specify for the fields. As if you just add a field outside a listing would be also collapsed as default.No?

Edit: But yes some predicate that would be form:isCollapsedOnRender; or the one you thought of (form:collapsedByDefault true;) seems good to :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe if we're not sure what the logic should be, it's better to first add the feature to make it collapsible and then later add defaults that we feel work best? As the feature is optional, it shouldn't get in the way. Non-collapsed also makes the behavior the same as when the form isn't collapsible.
WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me! Step by step 💪🏻

@aatauil aatauil marked this pull request as draft December 4, 2023 10:38
@aatauil
Copy link
Member

aatauil commented Dec 4, 2023

Converted to draft to prevent accidental merge

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.

3 participants